On Thu, Oct 7, 2010 at 2:52 PM, Benny Halevy <bhalevy@xxxxxxxxxxx> wrote: > On 2010-10-07 15:37, andros@xxxxxxxxxx wrote: >> From: Andy Adamson <andros@xxxxxxxxxxxxxxxxxxxxx> >> >> Do not get_lseg for a non-valid lseg. >> Prepare for calling put_lseg outside of inode i_lock. >> >> Signed-off-by: Andy Adamson <andros@xxxxxxxxxx> >> --- >> fs/nfs/pnfs.c | 4 ++-- >> 1 files changed, 2 insertions(+), 2 deletions(-) >> >> diff --git a/fs/nfs/pnfs.c b/fs/nfs/pnfs.c >> index 6b2a95d..24620cf 100644 >> --- a/fs/nfs/pnfs.c >> +++ b/fs/nfs/pnfs.c >> @@ -845,7 +845,8 @@ pnfs_has_layout(struct pnfs_layout_hdr *lo, >> list_for_each_entry(lseg, &lo->segs, fi_list) { >> if (is_matching_lseg(lseg, range)) { >> ret = lseg; >> - get_lseg(ret); >> + if (lseg->valid) >> + get_lseg(ret); > > We shouldn't be hiding this inside pnfs_has_layout > and have different side effects in the different cases. Sorry - I just looked at the pnfs-submit branch. I should have looked at the pnfs-all branch and seen the other callers of pnfs_has_layout. > Since we're called under the lock, pnfs_has_layout > can just return the lseg and never get a reference and its > caller can take it if necessary before releasing the lock. > > Also, it doesn't need to be EXPORT_SYMBOL_GPLed as it's > not used outside of the nfs client module. Right - Fred just removed the call in the file layout driver. I'll fix this. -->Andy > > Benny > >> break; >> } >> if (cmp_layout(range, &lseg->range) > 0) >> @@ -889,7 +890,6 @@ pnfs_update_layout(struct inode *ino, >> /* Check to see if the layout for the given range already exists */ >> lseg = pnfs_has_layout(lo, &arg); >> if (lseg && !lseg->valid) { >> - put_lseg_locked(lseg); >> /* someone is cleaning the layout */ >> lseg = NULL; >> goto out_unlock; > -- > To unsubscribe from this list: send the line "unsubscribe linux-nfs" in > the body of a message to majordomo@xxxxxxxxxxxxxxx > More majordomo info at http://vger.kernel.org/majordomo-info.html > -- To unsubscribe from this list: send the line "unsubscribe linux-nfs" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html