Re: [PATCH 1/6] nfs: "open code" the NFS direct write rescheduler

[Chuck Lever]

Andrew Morton wrote:
> Chuck Lever <cel@xxxxxxxxxxxxxx> wrote:
> > Andrew Morton wrote:
> > > Chuck Lever <cel@xxxxxxxxxx> wrote:
> > > > +	 * Prevent I/O completion while we're still rescheduling
> > > > +	 */
> > > > +	dreq->outstanding++;
> > > > +
> > > No locking.
> > >
> > > >  	dreq->count = 0;
> > > > +	list_for_each(pos, &dreq->rewrite_list) {
> > > > +		struct nfs_write_data *data =
> > > > +			list_entry(dreq->rewrite_list.next, struct nfs_write_data, pages);
> > > > +
> > > > +		spin_lock(&dreq->lock);
> > > > +		dreq->outstanding++;
> > > > +		spin_unlock(&dreq->lock);
> > > Locking.
> > >
> > > Deliberate?
> > Yes.  At the top of the loop, there is no outstanding I/O, so no locking 
> > is needed while updating "outstanding."  Inside the loop, we've 
> > dispatched some I/O against "dreq" so locking is needed to ensure 
> > outstanding is updated properly.
>
> OK.  Well if I asked, then others will wonder about it.  A comment would
> cure that problem ;)

Or, I could code defensively and just add locking there too, even though 
it is not needed.  This path is not a performance path, and things could 
get changed at some point so my assumption that is no longer valid.

--
corporate:	cel at netapp dot com
personal:	chucklever at bigfoot dot com
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/