Re: Checking of NULL with __GFP_NOFAIL in kzalloc()

[Theodore Tso <tytso@mit.edu>]

On Mon, Mar 02, 2009 at 11:38:53AM +0530, Manish Katiyar wrote:
> On Sun, Mar 1, 2009 at 11:35 AM, Manish Katiyar <mkatiyar@gmail.com> wrote:
> > Hi,
> >
> > While going through jbd code, I was wondering why do we need to check
> > new_transaction for NULL, if we are passing __GFP_NOFAIL ?
> > Last code change around this code was when Ted converted kmalloc to
> > kzalloc, but since he also didn't remove it I am guessing there would
> > be some good reason for it. Can someone enlighten me ?
> 
> I didn't receive any response to this. So probably removing the NULL
> check is harmless. Or should I remove the __GFP_NOFAIL flag and keep
> the error handling ?

We don't want to remove __GFP_NOFAIL here since returning an error
would cause the filesystem to be marked as corrupted, and so it's
better to simply ask the memory allocator to retry until it can
succeed, which is what GFP_NOFAIL does.

This isn't an excuse for using GFP_NOFAIL in journal_init_common(),
though; that's an example of a use of __GFP_NOFAIL that should go
away.  (Basically, if that fails, the mount will fail; we should
printk an explanatory message since the mount system call return
EINVAL, but failing a mount due to not enough memory is reasonable.
Marking the filesystem as corrupted, and remounting the filesystem
read-only and/or panic'ing the system is not.)

						- Ted