[0/8] ceph: cap handling code fixes, cleanups and comments

Message

Jeff Layton March 23, 2020, 4:07 p.m. UTC

I've been going over the cap handling code with an aim toward
simplifying the locking. There's one fix for a potential use-after-free
race in here. This also eliminates a number of __acquires and __releases
annotations by reorganizing the code, and adds some (hopefully helpful)
comments.

There should be no behavioral changes with this set.

Jeff Layton (8):
  ceph: reorganize __send_cap for less spinlock abuse
  ceph: split up __finish_cap_flush
  ceph: add comments for handle_cap_flush_ack logic
  ceph: don't release i_ceph_lock in handle_cap_trunc
  ceph: don't take i_ceph_lock in handle_cap_import
  ceph: document what protects i_dirty_item and i_flushing_item
  ceph: fix potential race in ceph_check_caps
  ceph: throw a warning if we destroy session with mutex still locked

 fs/ceph/caps.c       | 292 ++++++++++++++++++++++++-------------------
 fs/ceph/mds_client.c |   1 +
 fs/ceph/super.h      |   4 +-
 3 files changed, 170 insertions(+), 127 deletions(-)

Comments

Yan, Zheng March 24, 2020, 3:05 p.m. UTC | #1

On Tue, Mar 24, 2020 at 12:07 AM Jeff Layton <[e-mail]> wrote:
>
> I've been going over the cap handling code with an aim toward
> simplifying the locking. There's one fix for a potential use-after-free
> race in here. This also eliminates a number of __acquires and __releases
> annotations by reorganizing the code, and adds some (hopefully helpful)
> comments.
>
> There should be no behavioral changes with this set.
>
> Jeff Layton (8):
>   ceph: reorganize __send_cap for less spinlock abuse
>   ceph: split up __finish_cap_flush
>   ceph: add comments for handle_cap_flush_ack logic
>   ceph: don't release i_ceph_lock in handle_cap_trunc
>   ceph: don't take i_ceph_lock in handle_cap_import
>   ceph: document what protects i_dirty_item and i_flushing_item
>   ceph: fix potential race in ceph_check_caps
>   ceph: throw a warning if we destroy session with mutex still locked
>
>  fs/ceph/caps.c       | 292 ++++++++++++++++++++++++-------------------
>  fs/ceph/mds_client.c |   1 +
>  fs/ceph/super.h      |   4 +-
>  3 files changed, 170 insertions(+), 127 deletions(-)
>

Other than minor comment for the first commit, this series look good

> --
> 2.25.1
>

Luis Henriques March 31, 2020, 12:24 p.m. UTC | #2

On Mon, Mar 23, 2020 at 12:07:00PM -0400, Jeff Layton wrote:
> I've been going over the cap handling code with an aim toward
> simplifying the locking. There's one fix for a potential use-after-free
> race in here. This also eliminates a number of __acquires and __releases
> annotations by reorganizing the code, and adds some (hopefully helpful)
> comments.
> 
> There should be no behavioral changes with this set.

But a lot of clarifications!  Thanks a lot for this patchset, Jeff ;-)

Cheers,
--
Luis

> 
> Jeff Layton (8):
>   ceph: reorganize __send_cap for less spinlock abuse
>   ceph: split up __finish_cap_flush
>   ceph: add comments for handle_cap_flush_ack logic
>   ceph: don't release i_ceph_lock in handle_cap_trunc
>   ceph: don't take i_ceph_lock in handle_cap_import
>   ceph: document what protects i_dirty_item and i_flushing_item
>   ceph: fix potential race in ceph_check_caps
>   ceph: throw a warning if we destroy session with mutex still locked
> 
>  fs/ceph/caps.c       | 292 ++++++++++++++++++++++++-------------------
>  fs/ceph/mds_client.c |   1 +
>  fs/ceph/super.h      |   4 +-
>  3 files changed, 170 insertions(+), 127 deletions(-)
> 
> -- 
> 2.25.1
>