All of lore.kernel.org
 help / color / mirror / Atom feed
From: Manish Katiyar <mkatiyar@gmail.com>
To: "Ted Ts'o" <tytso@mit.edu>
Cc: Jan Kara <jack@suse.cz>, Andreas Dilger <adilger@dilger.ca>,
	linux-ext4@vger.kernel.org
Subject: Re: [PATCH 2/3] jbd2 : Fix journal start by passing a parameter to specify if the caller can deal with ENOMEM
Date: Thu, 26 May 2011 21:11:14 -0700	[thread overview]
Message-ID: <BANLkTikvZSPmO8bFsSh8QhSKK=OdPyGFUg@mail.gmail.com> (raw)
In-Reply-To: <20110526150846.GL9520@thunk.org>

On Thu, May 26, 2011 at 8:08 AM, Ted Ts'o <tytso@mit.edu> wrote:
> On Thu, May 26, 2011 at 04:49:56PM +0200, Jan Kara wrote:
>>   No need to do this. If you make JBD2 use a separate slab for transaction
>> structures (trivial and makes some sense anyway), you can use
>> fault-injection framework to do exactly what you describe above (see
>> Documentation/fault-injection/fault-injection.txt and look for failslab).
>
> Thanks for pointing me at the fault-injection framework; it's not
> something I've used before.  I'll have to take a look at it.
>
>>   But if we just fail all transaction allocations with say 10% probability,
>> it should work as well, shouldn't it? We'd just retry those allocations
>> whose failure we cannot handle and eventually succeed. Or do I miss
>> something?
>
> The reason why I only wanted to fail the transactions relating to the
> writeback path is because other failures will get reflected back to
> userspace, and would thus change the behavior of the stress test.  (If
> we used fsstress, it would cause fsstress to immediately stop and
> fail, for example.)
>
> That is the one thing that worries me a little about this patch series
> in general.  If we suddenly start failing open() or rename() or
> chmod() syscalls with ENOMEM in low memory situations, what of
> programs that aren't doing adequate error checking?  Sure, other file
> systems will do this, but the bulk of the users use ext3/ext4, and
> remember how much kvetching and complaining when xfs was the first
> file system to require user space applications to actually use fsync()
> if they wanted their files to be safe after a power failure.
>
> I worry that there are a lot of incompetently written editors out
> there that aren't doing error checking, or worse yet, package managers
> or other security-critical programs that aren't doing error checking,
> and which won't notice when an syscall fails in a low-memory
> situation, leading to either (a) user data loss (which the application
> programers will lay at the feet of the file system developers, don't
> doubt it), or (b) security holes.
>
> I'm not sure there's a way to address this concern, and I'm going not
> NACK'ing this patch series on that basis --- but I do worry that it
> might not improve the situation by a whole lot, and may in fact cause
> some problems, at the end of the day.

If there are applications which don't expect this behaviour or can't handle such
cases, we can always add a sysfs flag to turn of this behavior completely and
always retry like old behavior. This may be ugly, but would help till
ppl start realising
this behavior and writing applications appropriately.

-- 
Thanks -
Manish
--
To unsubscribe from this list: send the line "unsubscribe linux-ext4" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

  parent reply	other threads:[~2011-05-27  4:11 UTC|newest]

Thread overview: 14+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2011-05-25  7:26 [PATCH 2/3] jbd2 : Fix journal start by passing a parameter to specify if the caller can deal with ENOMEM Manish Katiyar
2011-05-25  7:44 ` Jan Kara
2011-05-25  7:47   ` Manish Katiyar
2011-05-25  8:13     ` Jan Kara
2011-05-26  2:22       ` Ted Ts'o
2011-05-26  4:07         ` Andreas Dilger
2011-05-26 14:05           ` Ted Ts'o
2011-05-26 14:49             ` Jan Kara
2011-05-26 15:08               ` Ted Ts'o
2011-05-26 15:37                 ` Jan Kara
2011-05-27  4:11                 ` Manish Katiyar [this message]
2011-05-26  5:29         ` Manish Katiyar
2011-05-26 14:51           ` Jan Kara
2011-05-28  6:16         ` Manish Katiyar

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to='BANLkTikvZSPmO8bFsSh8QhSKK=OdPyGFUg@mail.gmail.com' \
    --to=mkatiyar@gmail.com \
    --cc=adilger@dilger.ca \
    --cc=jack@suse.cz \
    --cc=linux-ext4@vger.kernel.org \
    --cc=tytso@mit.edu \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.