All of lore.kernel.org
 help / color / mirror / Atom feed
From: fanchaoting <fanchaoting@cn.fujitsu.com>
To: "Peng, Tao" <tao.peng@emc.com>
Cc: "linux-nfs@vger.kernel.org" <linux-nfs@vger.kernel.org>
Subject: Re: [PATCH] pnfs-block: may be return NULL when find a extent
Date: Wed, 13 Mar 2013 08:55:46 +0800	[thread overview]
Message-ID: <513FCE92.7050105@cn.fujitsu.com> (raw)
In-Reply-To: <1882637210201641A747464075FFC649025986A4@MX101CL02.corp.emc.com>

Peng, Tao 写道:
>> -----Original Message-----
>> From: linux-nfs-owner@vger.kernel.org [mailto:linux-nfs-owner@vger.kernel.org] On Behalf Of
>> fanchaoting
>> Sent: Tuesday, March 12, 2013 2:09 PM
>> To: Myklebust, Trond
>> Cc: linux-nfs@vger.kernel.org
>> Subject: [PATCH] pnfs-block: may be return NULL when find a extent
>>
>> maybe return NULL when find a extent, if we use it later,
>> it will cause oops.
>>
> ah, I would say it is intentional because the lookup cannot fail here. If we are committing an extent but cannot find it in client's extent cache, it certainly is a bug somewhere else. I remember there are other places like this in block layout code as well and we used to put a BUG_ON() there. But since we are using the pointer right away and crash on NULL pointer dereference anyway, it was suggested to remove these BUG_ON()s during code review.
> 

Get it , thanks for telling me it.

> Are you fixing this because you saw a crash or because some static code analyzer complains?
> 
> Thanks,
> Tao
> 
>>  Signed-off-by: fanchaoting<fanchaoting@cn.fujitsu.com>
>> ---
>>  fs/nfs/blocklayout/extents.c |    5 +++++
>>  1 files changed, 5 insertions(+), 0 deletions(-)
>>
>> diff --git a/fs/nfs/blocklayout/extents.c b/fs/nfs/blocklayout/extents.c
>> index 9c3e117..131ea59 100644
>> --- a/fs/nfs/blocklayout/extents.c
>> +++ b/fs/nfs/blocklayout/extents.c
>> @@ -762,6 +762,11 @@ set_to_rw(struct pnfs_block_layout *bl, u64 offset, u64 length)
>>
>>         spin_lock(&bl->bl_ext_lock);
>>         be = bl_find_get_extent_locked(bl, offset);
>> +       if (!be) {
>> +               spin_unlock(&bl->bl_ext_lock);
>> +               goto out_nosplit;
>> +       }
>> +
>>         rv = be->be_f_offset + be->be_length;
>>         if (be->be_state != PNFS_BLOCK_INVALID_DATA) {
>>                 spin_unlock(&bl->bl_ext_lock);
>> --
>> 1.7.1
>>
>> --
>> To unsubscribe from this list: send the line "unsubscribe linux-nfs" in
>> the body of a message to majordomo@vger.kernel.org
>> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> 




      reply	other threads:[~2013-03-13  0:55 UTC|newest]

Thread overview: 3+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2013-03-12  6:09 [PATCH] pnfs-block: may be return NULL when find a extent fanchaoting
2013-03-12 10:27 ` Peng, Tao
2013-03-13  0:55   ` fanchaoting [this message]

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=513FCE92.7050105@cn.fujitsu.com \
    --to=fanchaoting@cn.fujitsu.com \
    --cc=linux-nfs@vger.kernel.org \
    --cc=tao.peng@emc.com \
    /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.