lkml.org 
[lkml]   [2010]   [May]   [23]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
SubjectRe: [PATCH 14/27] drivers/scsi: Use memdup_user
From
Date
On Sun, 2010-05-23 at 18:36 +0300, Boaz Harrosh wrote:
> > This type of transformation has really no value at all. The code you're
> > proposing to replace is already correct. I'm fairly ambivalent on
> > patterned APIs anyway but I accept they're useful way to prevent new
> > code getting it wrong. However, it's completely bogus to force
> > replacement of correctly functioning code throughout the kernel (unless
> > you're going to argue that everyone who tries to copy from user into a
> > kmalloc space does a cut and paste from sg?)
> >
> > Of infinitely greater service would be finding any places where the
> > pattern is being incorrectly used.
> >
>
> It looks like it is not done 100% kosher and calling memdup_user should
> be better.
>
> - For 1 memdup_user does a GFP_KERNEL with a comment on how copy_from_user
> would eventually sleep, so what's the point of GFP_ATOMIC?

Well, since you've written a storage driver, I really hope that question
is rhetorical ...

The reason for using GFP_ATOMIC from user context in storage drivers is
to avoid writeout deadlock: you don't want to trigger a swap write
while you potentially occupy the writeout path. In all older drivers
this had to be GFP_ATOMIC because GFP_NOIO wasn't around.

This also illustrates the problem with design patterns: The idea that
if you have user context, you must be able to kmalloc GFP_KERNEL seems
logical to the people who wrote the pattern, but actually it's
potentially incorrect for storage.

Now in the particular case of sg, I don't believe we'll ever want to
swap over sg (famous last words, of course), so in this instance we
probably could get away with using GFP_KERNEL ... but since it's
following the storage pattern, GFP_ATOMIC (or GFP_NOIO) is correct.

Does osd need auditing for this problem (or would no-one ever do swap
over osd)?

> If this is by design then it surly calls for a comment that explains.
> (I would like to know)

This pattern occurs many times in storage ... documenting it at every
callsite would be a huge waste.

James




\
 
 \ /
  Last update: 2010-05-23 18:25    [from the cache]
©2003-2011 Jasper Spaans