[pnfs] [PATCH 5/6] pnfs: don't call put_lseg with lo_lock held

[William A. (Andy) Adamson]

On Thu, Jul 24, 2008 at 4:45 AM, Benny Halevy <bhalevy at panasas.com> wrote:
> On Wed, 23 Jul 2008 16:46:55 -0400 (EDT) Fredric Isaman <iisaman at citi.umich.edu> wrote:
>>
>>
>> On Wed, 23 Jul 2008, andros at netapp.com wrote:
>>
>> > From: Andy Adamson <andros at localhost.localdomain>
>> >
>> > We want to call put_current_layout from destroy_lseg.
>> >
>> > Signed-off-by: Andy Adamson<andros at netapp.com>
>> > ---
>> > fs/nfs/pnfs.c |   36 +++++++++++++++++++++++-------------
>> > 1 files changed, 23 insertions(+), 13 deletions(-)
>> >
>> > diff --git a/fs/nfs/pnfs.c b/fs/nfs/pnfs.c
>> > index d5f2b07..e629984 100644
>> > --- a/fs/nfs/pnfs.c
>> > +++ b/fs/nfs/pnfs.c
>> > @@ -561,15 +561,22 @@ free_matching_lseg(struct pnfs_layout_segment *lseg,
>> >            lo_seg_intersecting(&lseg->range, range);
>> > }
>> >
>> > -static void
>> > -pnfs_free_layout(struct pnfs_layout_type *lo,
>> > -            struct nfs4_pnfs_layout_segment *range)
>> > +static struct pnfs_layout_type *
>> > +pnfs_free_layout(struct inode *ino, struct nfs4_pnfs_layout_segment *range)
>> > {
>> >     struct pnfs_layout_segment *lseg, *next;
>> > -   dprintk("%s:Begin lo %p offset %llu length %llu iomode %d\n",
>> > +   struct pnfs_layout_type *lo = NULL;
>> > +   struct nfs_inode *nfsi = NFS_I(ino);
>> > +   int status = 0;
>> > +
>> > +   dprintk("--> %s lo %p offset %llu length %llu iomode %d\n",
>> >             __func__, lo, range->offset, range->length, range->iomode);
>> >
>> > -   BUG_ON_UNLOCKED_LO(lo);
>> > +   lo = get_lock_current_layout(nfsi);
>> > +   if (lo == NULL) {
>> > +           lo  = ERR_PTR(-EIO);
>> > +           goto out;
>> > +   }
>> >     list_for_each_entry_safe (lseg, next, &lo->segs, fi_list) {
>> >             if (!free_matching_lseg(lseg, range))
>> >                     continue;
>> > @@ -578,10 +585,15 @@ pnfs_free_layout(struct pnfs_layout_type *lo,
>> >                     lseg, lseg->range.iomode, lseg->range.offset,
>> >                     lseg->range.length);
>> >             list_del(&lseg->fi_list);
>> > +           spin_unlock(&nfsi->lo_lock);
>> >             put_lseg(lseg);
>> > +           goto out;
>>
>> Breaking out of the loop seems a major unrelated behavior change.  Why?
>
> free_layout should free all matching segments.
> When there are multiple overlapping layout segments there
> could indeed be several of them.


OK. I can fix this.

-->Andy

> Beny
>
>>
>> Fred
>>
>> >     }
>> > +   spin_unlock(&nfsi->lo_lock);
>> >
>> > -   dprintk("%s:Return\n", __func__);
>> > +out:
>> > +   dprintk("<-- %s status %d\n", __func__, status);
>> > +   return lo;
>> > }
>> >
>> > static int
>> > @@ -616,9 +628,8 @@ int
>> > _pnfs_return_layout(struct inode *ino, struct nfs4_pnfs_layout_segment *range,
>> >                 enum pnfs_layoutrecall_type type)
>> > {
>> > -   struct pnfs_layout_type *lo = NULL;
>> > -   struct nfs_inode *nfsi = NFS_I(ino);
>> >     struct nfs4_pnfs_layout_segment arg;
>> > +   struct pnfs_layout_type *lo;
>> >     int status;
>> >
>> >     dprintk("--> %s type %d\n", __func__, type);
>> > @@ -630,13 +641,12 @@ _pnfs_return_layout(struct inode *ino, struct nfs4_pnfs_layout_segment *range,
>> >             arg.offset = 0;
>> >             arg.length = ~0;
>> >     }
>> > -   lo = get_lock_current_layout(nfsi);
>> > -   if (lo == NULL) {
>> > -           status = -EIO;
>> > +
>> > +   lo = pnfs_free_layout(ino, &arg);
>> > +   if (IS_ERR(lo)) {
>> > +           status = PTR_ERR(lo);
>> >             goto out;
>> >     }
>> > -   pnfs_free_layout(lo, &arg);
>> > -   spin_unlock(&nfsi->lo_lock);
>> >
>> >     status = return_layout(ino, &arg, type, lo);
>> > out:
>> > --
>> > 1.5.4.1
>> >
>> > _______________________________________________
>> > pNFS mailing list
>> > pNFS at linux-nfs.org
>> > http://linux-nfs.org/cgi-bin/mailman/listinfo/pnfs
>> >
>> _______________________________________________
>> pNFS mailing list
>> pNFS at linux-nfs.org
>> http://linux-nfs.org/cgi-bin/mailman/listinfo/pnfs
> _______________________________________________
> pNFS mailing list
> pNFS at linux-nfs.org
> http://linux-nfs.org/cgi-bin/mailman/listinfo/pnfs
>
>