Re: [PATCH v7 2/2] ceph: blocklist the kclient when receiving corrupted snap trace

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Thu, Feb 2, 2023 at 3:30 AM Xiubo Li <xiubli@xxxxxxxxxx> wrote:
>
>
> On 01/02/2023 21:12, Ilya Dryomov wrote:
> > On Wed, Feb 1, 2023 at 2:37 AM <xiubli@xxxxxxxxxx> wrote:
> >> From: Xiubo Li <xiubli@xxxxxxxxxx>
> >>
> >> When received corrupted snap trace we don't know what exactly has
> >> happened in MDS side. And we shouldn't continue IOs and metadatas
> >> access to MDS, which may corrupt or get incorrect contents.
> >>
> >> This patch will just block all the further IO/MDS requests
> >> immediately and then evict the kclient itself.
> >>
> >> The reason why we still need to evict the kclient just after
> >> blocking all the further IOs is that the MDS could revoke the caps
> >> faster.
> >>
> >> Cc: stable@xxxxxxxxxxxxxxx
> >> URL: https://tracker.ceph.com/issues/57686
> >> Reviewed-by: Venky Shankar <vshankar@xxxxxxxxxx>
> >> Signed-off-by: Xiubo Li <xiubli@xxxxxxxxxx>
> >> ---
> >>   fs/ceph/addr.c       | 17 +++++++++++++++--
> >>   fs/ceph/caps.c       | 16 +++++++++++++---
> >>   fs/ceph/file.c       |  9 +++++++++
> >>   fs/ceph/mds_client.c | 28 +++++++++++++++++++++++++---
> >>   fs/ceph/snap.c       | 38 ++++++++++++++++++++++++++++++++++++--
> >>   fs/ceph/super.h      |  1 +
> >>   6 files changed, 99 insertions(+), 10 deletions(-)
> >>
> >> diff --git a/fs/ceph/addr.c b/fs/ceph/addr.c
> >> index 8c74871e37c9..cac4083e387a 100644
> >> --- a/fs/ceph/addr.c
> >> +++ b/fs/ceph/addr.c
> >> @@ -305,7 +305,7 @@ static void ceph_netfs_issue_read(struct netfs_io_subrequest *subreq)
> >>          struct inode *inode = rreq->inode;
> >>          struct ceph_inode_info *ci = ceph_inode(inode);
> >>          struct ceph_fs_client *fsc = ceph_inode_to_client(inode);
> >> -       struct ceph_osd_request *req;
> >> +       struct ceph_osd_request *req = NULL;
> >>          struct ceph_vino vino = ceph_vino(inode);
> >>          struct iov_iter iter;
> >>          struct page **pages;
> >> @@ -313,6 +313,11 @@ static void ceph_netfs_issue_read(struct netfs_io_subrequest *subreq)
> >>          int err = 0;
> >>          u64 len = subreq->len;
> >>
> >> +       if (ceph_inode_is_shutdown(inode)) {
> >> +               err = -EIO;
> >> +               goto out;
> >> +       }
> >> +
> >>          if (ceph_has_inline_data(ci) && ceph_netfs_issue_op_inline(subreq))
> >>                  return;
> >>
> >> @@ -563,6 +568,9 @@ static int writepage_nounlock(struct page *page, struct writeback_control *wbc)
> >>
> >>          dout("writepage %p idx %lu\n", page, page->index);
> >>
> >> +       if (ceph_inode_is_shutdown(inode))
> >> +               return -EIO;
> >> +
> >>          /* verify this is a writeable snap context */
> >>          snapc = page_snap_context(page);
> >>          if (!snapc) {
> >> @@ -1643,7 +1651,7 @@ int ceph_uninline_data(struct file *file)
> >>          struct ceph_inode_info *ci = ceph_inode(inode);
> >>          struct ceph_fs_client *fsc = ceph_inode_to_client(inode);
> >>          struct ceph_osd_request *req = NULL;
> >> -       struct ceph_cap_flush *prealloc_cf;
> >> +       struct ceph_cap_flush *prealloc_cf = NULL;
> >>          struct folio *folio = NULL;
> >>          u64 inline_version = CEPH_INLINE_NONE;
> >>          struct page *pages[1];
> >> @@ -1657,6 +1665,11 @@ int ceph_uninline_data(struct file *file)
> >>          dout("uninline_data %p %llx.%llx inline_version %llu\n",
> >>               inode, ceph_vinop(inode), inline_version);
> >>
> >> +       if (ceph_inode_is_shutdown(inode)) {
> >> +               err = -EIO;
> >> +               goto out;
> >> +       }
> >> +
> >>          if (inline_version == CEPH_INLINE_NONE)
> >>                  return 0;
> >>
> >> diff --git a/fs/ceph/caps.c b/fs/ceph/caps.c
> >> index f75ad432f375..210e40037881 100644
> >> --- a/fs/ceph/caps.c
> >> +++ b/fs/ceph/caps.c
> >> @@ -4078,6 +4078,7 @@ void ceph_handle_caps(struct ceph_mds_session *session,
> >>          void *p, *end;
> >>          struct cap_extra_info extra_info = {};
> >>          bool queue_trunc;
> >> +       bool close_sessions = false;
> >>
> >>          dout("handle_caps from mds%d\n", session->s_mds);
> >>
> >> @@ -4215,9 +4216,13 @@ void ceph_handle_caps(struct ceph_mds_session *session,
> >>                  realm = NULL;
> >>                  if (snaptrace_len) {
> >>                          down_write(&mdsc->snap_rwsem);
> >> -                       ceph_update_snap_trace(mdsc, snaptrace,
> >> -                                              snaptrace + snaptrace_len,
> >> -                                              false, &realm);
> >> +                       if (ceph_update_snap_trace(mdsc, snaptrace,
> >> +                                                  snaptrace + snaptrace_len,
> >> +                                                  false, &realm)) {
> >> +                               up_write(&mdsc->snap_rwsem);
> >> +                               close_sessions = true;
> >> +                               goto done;
> >> +                       }
> >>                          downgrade_write(&mdsc->snap_rwsem);
> >>                  } else {
> >>                          down_read(&mdsc->snap_rwsem);
> >> @@ -4277,6 +4282,11 @@ void ceph_handle_caps(struct ceph_mds_session *session,
> >>          iput(inode);
> >>   out:
> >>          ceph_put_string(extra_info.pool_ns);
> >> +
> >> +       /* Defer closing the sessions after s_mutex lock being released */
> >> +       if (close_sessions)
> >> +               ceph_mdsc_close_sessions(mdsc);
> >> +
> >>          return;
> >>
> >>   flush_cap_releases:
> >> diff --git a/fs/ceph/file.c b/fs/ceph/file.c
> >> index 764598e1efd9..3fbf4993d15d 100644
> >> --- a/fs/ceph/file.c
> >> +++ b/fs/ceph/file.c
> >> @@ -943,6 +943,9 @@ static ssize_t ceph_sync_read(struct kiocb *iocb, struct iov_iter *to,
> >>          dout("sync_read on file %p %llu~%u %s\n", file, off, (unsigned)len,
> >>               (file->f_flags & O_DIRECT) ? "O_DIRECT" : "");
> >>
> >> +       if (ceph_inode_is_shutdown(inode))
> >> +               return -EIO;
> > Hi Xiubo,
> >
> > ceph_sync_read() is called only from ceph_read_iter() which already
> > checks for ceph_inode_is_shutdown() (although the generated error is
> > ESTALE instead of EIO).  Is the new one in ceph_sync_read() actually
> > needed?
> >
> > If the answer is yes, why hasn't a corresponding check been added to
> > ceph_sync_write()?
>
> Before I generated this patch based on the fscrypt patches,  this will
> be renamed to __ceph_sync_read() and also will be called by
> fill_fscrypt_truncate(). I just forgot this and after rebasing I didn't
> adjust it.
>
> I have updated the 'wip-snap-trace-blocklist' branch by removing it from
> here and ceph_direct_read_write(). And I will fix this later in the
> fscrypt patches.

Hi Xiubo,

The latest revision looks fine.  I folded the "ceph: dump the msg when
receiving a corrupt snap trace" follow-up into this patch and pulled the
result into master.

I also rebased testing appropriately, feel free to perform the necessary
fscrypt-related fixups there!

Thanks,

                Ilya



[Index of Archives]     [CEPH Users]     [Ceph Large]     [Ceph Dev]     [Information on CEPH]     [Linux BTRFS]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]

  Powered by Linux