Re: [PATCH] Fix kunmap() argument in sg_miter_stop

[Ingo Molnar]

* Jens Axboe <jens.axboe@xxxxxxxxxx> wrote:

> On Mon, Nov 17 2008, Linus Torvalds wrote:
> > 
> > 
> > On Mon, 17 Nov 2008, Jens Axboe wrote:
> > > 
> > > Any opinions on the kunmap/kunmap_atomic pointer checking? It's a bit
> > > ugly that we have to enforce a void * rule for kunmap_atomic(),
> > 
> > I don't think that's a "bit ugly". I think it's unacceptable.
> > 
> > Making sure we pass in "struct page" to kunmap() sounds good, but the 
> > kunmap_atomic() part just sounds insane.
> 
> It's been the primary source of bugs that I have seen. The xen and 
> sg iter bug were kunmap() variants though, but otherwise I've mostly 
> seen the opposite. But it is ugly, no doubt about it. I can't think 
> of a better way to attempt to warn about it though, so if you really 
> dislike it I'll just drop the _atomic() bits.

The main ugliness comes from the tons of void * type casts that the 
kunmap_atomic() type check forces. Type casts are just as dangerous 
(and ugly) as type mismatches. (more dangerous in fact)

Perhaps we could try an opt-in 'type filter' approach instead. See 
kernel/tracing/trace.h's trace_assign_type()'s type checking magic for 
an example of how to do it.

( but it's a bit tricky here because we want to filter void * from
  struct page * - i'm not sure gcc will recognize them as incompatible
  types. )

	Ingo
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/