On Wed, 27 May 2009 09:40:43 -0400 Christoph Hellwig <hch@xxxxxxxxxxxxx> wrote: > Looks good. > > > Reviewed-by: Christoph Hellwig <hch@xxxxxx> > > Two little nitpicks: > > On Wed, May 27, 2009 at 08:30:29AM -0400, Jeff Layton wrote: > > access_flags_to_mode(ppace[i]->access_req, > > ppace[i]->type, > > - &(inode->i_mode), > > + &(fattr->cf_mode), > > &user_mask); > > if (compare_sids(&(ppace[i]->sid), pgrpsid)) > > access_flags_to_mode(ppace[i]->access_req, > > ppace[i]->type, > > - &(inode->i_mode), > > + &(fattr->cf_mode), > > &group_mask); > > if (compare_sids(&(ppace[i]->sid), &sid_everyone)) > > access_flags_to_mode(ppace[i]->access_req, > > ppace[i]->type, > > - &(inode->i_mode), > > + &(fattr->cf_mode), > > &other_mask); > > If you touch these lines please also remove the superflous braces. > > > + cifs_i->delete_pending = fattr->cf_flags & CIFS_FATTR_DELETE_PENDING; > > + > > + /* > > + * Can't safely change the file size here if the client is writing to > > + * it due to potential races. > > + */ > > spin_lock(&inode->i_lock); > > if (is_size_safe_to_change(cifs_i, fattr->cf_eof)) { > > - /* > > - * We can not safely change the file size here if the client > > - * is writing to it due to potential races. > > - */ > > Why isn't this comment introduced in the correct location in the > patch adding it? > Thanks for the review. I'll fix the above and will repost the set soon. I'll also plan to consolidate more of the patches for the next set to try and keep things bisectable. -- Jeff Layton <jlayton@xxxxxxxxxx> -- To unsubscribe from this list: send the line "unsubscribe linux-fsdevel" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html