Message ID | 20210506143312.22281-1-idryomov@gmail.com (mailing list archive) |
---|---|
State | New, archived |
Headers | show |
Series | [GIT,PULL] Ceph updates for 5.13-rc1 | expand |
On Thu, May 6, 2021 at 7:33 AM Ilya Dryomov <idryomov@gmail.com> wrote: > > There is a merge conflict in fs/ceph/dir.c because Jeff's inode > type handling patch went through the vfs tree together with Al's > inode_wrong_type() helper. for-linus-merged has the resolution. Actually, the linux-next resolution looks wrong - or at least unnecessary - to me. The conversion to d_splice_alias() means that the IS_ERR() test is now pointless, because d_splice_alias() handles an error-pointer natively, and just returns the error back with ERR_CAST(). So the proper resolution seems to be to just drop the IS_ERR(). Adding Jeff and Al just as a heads-up. Linus
The pull request you sent on Thu, 6 May 2021 16:33:12 +0200:
> https://github.com/ceph/ceph-client.git tags/ceph-for-5.13-rc1
has been merged into torvalds/linux.git:
https://git.kernel.org/torvalds/c/7ac86b3dca1b00f5391d346fdea3ac010d230667
Thank you!
On Thu, May 06, 2021 at 10:51:33AM -0700, Linus Torvalds wrote: > On Thu, May 6, 2021 at 7:33 AM Ilya Dryomov <idryomov@gmail.com> wrote: > > > > There is a merge conflict in fs/ceph/dir.c because Jeff's inode > > type handling patch went through the vfs tree together with Al's > > inode_wrong_type() helper. for-linus-merged has the resolution. > > Actually, the linux-next resolution looks wrong - or at least > unnecessary - to me. > > The conversion to d_splice_alias() means that the IS_ERR() test is now > pointless, because d_splice_alias() handles an error-pointer natively, > and just returns the error back with ERR_CAST(). > > So the proper resolution seems to be to just drop the IS_ERR(). Agreed; -next resolution is not wrong per se, but it's not needed - d_splice_alias(ERR_PTR(e), d) == ERR_PTR(e) for any e in -4095..-1, so the variant of resolution in mainline merge will do the right thing.
On Thu, May 6, 2021 at 7:51 PM Linus Torvalds <torvalds@linux-foundation.org> wrote: > > On Thu, May 6, 2021 at 7:33 AM Ilya Dryomov <idryomov@gmail.com> wrote: > > > > There is a merge conflict in fs/ceph/dir.c because Jeff's inode > > type handling patch went through the vfs tree together with Al's > > inode_wrong_type() helper. for-linus-merged has the resolution. > > Actually, the linux-next resolution looks wrong - or at least > unnecessary - to me. > > The conversion to d_splice_alias() means that the IS_ERR() test is now > pointless, because d_splice_alias() handles an error-pointer natively, > and just returns the error back with ERR_CAST(). > > So the proper resolution seems to be to just drop the IS_ERR(). > > Adding Jeff and Al just as a heads-up. I did it mechanically and then cross-checked against Jeff's patch: https://lore.kernel.org/ceph-devel/20210316203919.102346-1-jlayton@kernel.org/T/#u I guess neither Jeff nor I noticed that ERR_CAST() is redundant because previously ceph_get_snapdir() didn't have any error handling and the explicit check (which Jeff added in another patch that went through Al's tree) felt "precious". Thanks, Ilya
On Fri, 2021-05-07 at 11:03 +0200, Ilya Dryomov wrote: > On Thu, May 6, 2021 at 7:51 PM Linus Torvalds > <torvalds@linux-foundation.org> wrote: > > > > On Thu, May 6, 2021 at 7:33 AM Ilya Dryomov <idryomov@gmail.com> wrote: > > > > > > There is a merge conflict in fs/ceph/dir.c because Jeff's inode > > > type handling patch went through the vfs tree together with Al's > > > inode_wrong_type() helper. for-linus-merged has the resolution. > > > > Actually, the linux-next resolution looks wrong - or at least > > unnecessary - to me. > > > > The conversion to d_splice_alias() means that the IS_ERR() test is now > > pointless, because d_splice_alias() handles an error-pointer natively, > > and just returns the error back with ERR_CAST(). > > > > So the proper resolution seems to be to just drop the IS_ERR(). > > > > Adding Jeff and Al just as a heads-up. > > I did it mechanically and then cross-checked against Jeff's patch: > > https://lore.kernel.org/ceph-devel/20210316203919.102346-1-jlayton@kernel.org/T/#u > > I guess neither Jeff nor I noticed that ERR_CAST() is redundant > because previously ceph_get_snapdir() didn't have any error handling > and the explicit check (which Jeff added in another patch that went > through Al's tree) felt "precious". > Yep, I missed that special error handling too. The final fixup looks fine. Cheers,