From: Glauber de Oliveira Costa <glommer@br.ibm.com>
To: Mikulas Patocka <mikulas@artax.karlin.mff.cuni.cz>
Cc: glommer@br.ibm.com, Anton Altaparmakov <aia21@cam.ac.uk>,
linux-kernel@vger.kernel.org, linux-fsdevel@vger.kernel.org,
ext2-devel@lists.sourceforge.net, hirofumi@mail.parknet.co.jp,
linux-ntfs-dev@lists.sourceforge.net, aia21@cantab.net,
hch@infradead.org, viro@zeniv.linux.org.uk, akpm@osdl.org
Subject: Re: [PATCH] Use of getblk differs between locations
Date: Mon, 10 Oct 2005 19:36:36 -0300 [thread overview]
Message-ID: <20051010223636.GB11427@br.ibm.com> (raw)
In-Reply-To: <Pine.LNX.4.62.0510102347220.19021@artax.karlin.mff.cuni.cz>
> >>If you had read the source code rather than just the comments you would
> >>have seen that this is not true. It can return NULL (see
> >>fs/buffer.c::__getblk_slow()). Certainly I would prefer to keep the
> >>checks in NTFS, please. They may only be good for catching bugs but I
> >>like catching bugs rather than segfaulting due to a NULL dereference.
>
> The check should be rather a BUG() than dump_stack() and return NULL --- I
> think it's not right to write code to recover from programming errors.
> Filesystem drivers are supposed to pass correct blocksize to getblk(). ---
> even for users it's better to crash, because user whose machine has locked
> up on BUG() will report bug more likely than user whose machine has
> written stack dump into log and corrupted filesystem --- by the time he
> discovers the corruption and mesage he might not even remember what
> triggered it.
That was what I meant by having the opposite problem here. I think
dumping the stack and returning NULL is okay, as long as all programmers
test its return value, and decide to fail in an alternative way, just
like Anton does, for example. But unfortunately, that's not what happen.
In a lot of cases, we see uses like these: (This one from affs.h)
bh = sb_getblk(sb, block);
lock_buffer(bh);
memset(bh->b_data, 0 , sb->s_blocksize);
set_buffer_uptodate(bh);
unlock_buffer(bh);
Which does not seem to be the right usage for it.
As I said, I took away the checks because I missed that return
statement. I usually don't think that hanging is the preferred solution
in the cases in which you can stop gracefully - But in case you do stop
gracefully, not dereference a NULL pointer.
>
> As comment in buffer.c says, getblk will deadlock if the machine is out of
> memory. It is questionable whether to deadlock or return NULL and corrupt
> filesystem in this case --- deadlock is probably better.
>
> Mikulas
Maybe the best solution is neither one nor another. Testing and failing
gracefully seems better.
What do you think?
Glauber
next prev parent reply other threads:[~2005-10-10 22:26 UTC|newest]
Thread overview: 42+ messages / expand[flat|nested] mbox.gz Atom feed top
2005-10-10 20:45 Glauber de Oliveira Costa
2005-10-10 21:20 ` Anton Altaparmakov
2005-10-10 21:46 ` Glauber de Oliveira Costa
2005-10-10 21:58 ` Mikulas Patocka
2005-10-10 22:25 ` Anton Altaparmakov
2005-10-10 22:49 ` Mikulas Patocka
2005-10-10 23:12 ` Glauber de Oliveira Costa
2005-10-10 23:16 ` Mikulas Patocka
2005-10-10 23:33 ` Glauber de Oliveira Costa
2005-10-10 23:34 ` Mikulas Patocka
2005-10-10 23:49 ` Glauber de Oliveira Costa
2005-10-11 7:52 ` Anton Altaparmakov
2005-10-12 19:51 ` Jeff Mahoney
2005-10-12 19:59 ` Mikulas Patocka
2005-10-12 20:07 ` Jeff Mahoney
2005-10-12 20:12 ` Mikulas Patocka
2005-10-12 20:14 ` Anton Altaparmakov
2005-10-12 20:31 ` Mikulas Patocka
2005-10-12 21:19 ` Jeff Mahoney
2005-10-12 21:35 ` Anton Altaparmakov
2005-10-13 0:09 ` Jamie Lokier
2005-10-13 0:21 ` Mikulas Patocka
2005-10-13 0:27 ` Jamie Lokier
2005-10-13 11:17 ` Pavel Machek
2005-10-14 16:52 ` Jamie Lokier
2005-10-14 18:26 ` Mikulas Patocka
2005-10-13 0:05 ` Jamie Lokier
2005-10-12 20:08 ` Anton Altaparmakov
2005-10-10 22:36 ` Glauber de Oliveira Costa [this message]
2005-10-10 22:28 ` Anton Altaparmakov
2005-10-10 23:36 ` Andrew Morton
2005-10-11 0:07 ` Glauber de Oliveira Costa
2005-10-11 0:05 ` Al Viro
2005-10-11 0:40 ` Glauber de Oliveira Costa
2005-10-11 12:35 ` Jan Hudec
2005-10-11 0:09 ` Mikulas Patocka
2005-10-11 1:07 ` Andrew Morton
2005-10-11 1:20 ` Mikulas Patocka
2005-10-11 5:02 ` Andrew Morton
2005-10-11 8:07 ` Anton Altaparmakov
2005-10-11 8:01 ` Anton Altaparmakov
2005-10-13 0:58 ` Mike Christie
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=20051010223636.GB11427@br.ibm.com \
--to=glommer@br.ibm.com \
--cc=aia21@cam.ac.uk \
--cc=aia21@cantab.net \
--cc=akpm@osdl.org \
--cc=ext2-devel@lists.sourceforge.net \
--cc=hch@infradead.org \
--cc=hirofumi@mail.parknet.co.jp \
--cc=linux-fsdevel@vger.kernel.org \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-ntfs-dev@lists.sourceforge.net \
--cc=mikulas@artax.karlin.mff.cuni.cz \
--cc=viro@zeniv.linux.org.uk \
/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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox
Powered by JetHome