On 08/25/2014 10:09 AM, Christoph Hellwig wrote:
> On Mon, Aug 25, 2014 at 09:50:34AM -0400, Anna Schumaker wrote:
>>> + } else {
>>> + if (NFS_SERVER(ino)->pnfs_curr_ld->return_range) {
>>> + NFS_SERVER(ino)->pnfs_curr_ld->return_range(lo,
>>> + &args->cbl_range);
>>> + }
>>> }
>> Is there a reason you're nesting the else-if here?
>
> To catch the intent - the first two clauses find excuses why we can't return
> quite yet, while this if is for an optional feature in the actual return
> path. If I wasn't updating but newly writing the function I'd actually
> do something like:
I'm a fan of nice looking code, and I like what you have below better. Can you arrange things to end up in this state? Or maybe send a cleanup patch after?
Anna
>
> ...
>
> rv = NFS4ERR_DELAY;
> if (test_bit(NFS_LAYOUT_BULK_RECALL, &lo->plh_flags))
> goto out_set_stateid;
>
> if (pnfs_mark_matching_lsegs_invalid(lo, &free_me_list,
> &args->cbl_range)) {
> need_commit = true;
> goto out_set_stateid;
> }
>
> rv = NFS4ERR_NOMATCHING_LAYOUT;
> if (NFS_SERVER(ino)->pnfs_curr_ld->return_range) {
> NFS_SERVER(ino)->pnfs_curr_ld->return_range(lo,
> &args->cbl_range);
> }
>
> out_set_stateid:
> ...
>
--
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
