From: Dan Williams <dan.j.williams@intel.com> To: Shiyang Ruan <ruansy.fnst@fujitsu.com> Cc: Linux Kernel Mailing List <linux-kernel@vger.kernel.org>, linux-xfs <linux-xfs@vger.kernel.org>, Linux NVDIMM <nvdimm@lists.linux.dev>, Linux MM <linux-mm@kvack.org>, linux-fsdevel <linux-fsdevel@vger.kernel.org>, device-mapper development <dm-devel@redhat.com>, "Darrick J. Wong" <djwong@kernel.org>, david <david@fromorbit.com>, Christoph Hellwig <hch@lst.de>, Alasdair Kergon <agk@redhat.com>, Mike Snitzer <snitzer@redhat.com> Subject: Re: [PATCH RESEND v6 6/9] xfs: Implement ->notify_failure() for XFS Date: Fri, 20 Aug 2021 15:59:02 -0700 [thread overview] Message-ID: <CAPcyv4h8eUKYDz+KLzXeMTEKc03k=8juXtYjYj+XSVQ5ww=KyQ@mail.gmail.com> (raw) In-Reply-To: <20210730100158.3117319-7-ruansy.fnst@fujitsu.com> On Fri, Jul 30, 2021 at 3:02 AM Shiyang Ruan <ruansy.fnst@fujitsu.com> wrote: > > This function is used to handle errors which may cause data lost in > filesystem. Such as memory failure in fsdax mode. > > If the rmap feature of XFS enabled, we can query it to find files and > metadata which are associated with the corrupt data. For now all we do > is kill processes with that file mapped into their address spaces, but > future patches could actually do something about corrupt metadata. > > After that, the memory failure needs to notify the processes who are > using those files. > > Signed-off-by: Shiyang Ruan <ruansy.fnst@fujitsu.com> > --- > drivers/dax/super.c | 12 ++++ > fs/xfs/xfs_fsops.c | 5 ++ > fs/xfs/xfs_mount.h | 1 + > fs/xfs/xfs_super.c | 135 ++++++++++++++++++++++++++++++++++++++++++++ > include/linux/dax.h | 13 +++++ > 5 files changed, 166 insertions(+) > > diff --git a/drivers/dax/super.c b/drivers/dax/super.c > index 00c32dfa5665..63f7b63d078d 100644 > --- a/drivers/dax/super.c > +++ b/drivers/dax/super.c > @@ -65,6 +65,18 @@ struct dax_device *fs_dax_get_by_bdev(struct block_device *bdev) > return dax_get_by_host(bdev->bd_disk->disk_name); > } > EXPORT_SYMBOL_GPL(fs_dax_get_by_bdev); > + > +void fs_dax_set_holder(struct dax_device *dax_dev, void *holder, > + const struct dax_holder_operations *ops) > +{ > + dax_set_holder(dax_dev, holder, ops); > +} > +EXPORT_SYMBOL_GPL(fs_dax_set_holder); Small style issue, I'd prefer a pair of functions: fs_dax_register_holder(struct dax_device *dax_dev, void *holder, const struct dax_holder_operations *ops) fs_dax_unregister_holder(struct dax_device *dax_dev) ...rather than open coding unregister as a special set that passes NULL arguments. > +void *fs_dax_get_holder(struct dax_device *dax_dev) > +{ > + return dax_get_holder(dax_dev); Does dax_get_holder() have a lockdep_assert to check that the caller has at least a read_lock? Please add kernel-doc for this api to indicate the locking context expectations. The rest of this looks plausibly ok to me, but it would be up to xfs folks to comment on the details. I'm not entirely comfortable with these handlers assuming DAX, i.e. they should also one day be useful for page cache memory failure notifications, but that support can come later.
WARNING: multiple messages have this Message-ID (diff)
From: Dan Williams <dan.j.williams@intel.com> To: Shiyang Ruan <ruansy.fnst@fujitsu.com> Cc: Linux NVDIMM <nvdimm@lists.linux.dev>, Mike Snitzer <snitzer@redhat.com>, "Darrick J. Wong" <djwong@kernel.org>, david <david@fromorbit.com>, Linux Kernel Mailing List <linux-kernel@vger.kernel.org>, linux-xfs <linux-xfs@vger.kernel.org>, Linux MM <linux-mm@kvack.org>, device-mapper development <dm-devel@redhat.com>, linux-fsdevel <linux-fsdevel@vger.kernel.org>, Christoph Hellwig <hch@lst.de>, Alasdair Kergon <agk@redhat.com> Subject: Re: [dm-devel] [PATCH RESEND v6 6/9] xfs: Implement ->notify_failure() for XFS Date: Fri, 20 Aug 2021 15:59:02 -0700 [thread overview] Message-ID: <CAPcyv4h8eUKYDz+KLzXeMTEKc03k=8juXtYjYj+XSVQ5ww=KyQ@mail.gmail.com> (raw) In-Reply-To: <20210730100158.3117319-7-ruansy.fnst@fujitsu.com> On Fri, Jul 30, 2021 at 3:02 AM Shiyang Ruan <ruansy.fnst@fujitsu.com> wrote: > > This function is used to handle errors which may cause data lost in > filesystem. Such as memory failure in fsdax mode. > > If the rmap feature of XFS enabled, we can query it to find files and > metadata which are associated with the corrupt data. For now all we do > is kill processes with that file mapped into their address spaces, but > future patches could actually do something about corrupt metadata. > > After that, the memory failure needs to notify the processes who are > using those files. > > Signed-off-by: Shiyang Ruan <ruansy.fnst@fujitsu.com> > --- > drivers/dax/super.c | 12 ++++ > fs/xfs/xfs_fsops.c | 5 ++ > fs/xfs/xfs_mount.h | 1 + > fs/xfs/xfs_super.c | 135 ++++++++++++++++++++++++++++++++++++++++++++ > include/linux/dax.h | 13 +++++ > 5 files changed, 166 insertions(+) > > diff --git a/drivers/dax/super.c b/drivers/dax/super.c > index 00c32dfa5665..63f7b63d078d 100644 > --- a/drivers/dax/super.c > +++ b/drivers/dax/super.c > @@ -65,6 +65,18 @@ struct dax_device *fs_dax_get_by_bdev(struct block_device *bdev) > return dax_get_by_host(bdev->bd_disk->disk_name); > } > EXPORT_SYMBOL_GPL(fs_dax_get_by_bdev); > + > +void fs_dax_set_holder(struct dax_device *dax_dev, void *holder, > + const struct dax_holder_operations *ops) > +{ > + dax_set_holder(dax_dev, holder, ops); > +} > +EXPORT_SYMBOL_GPL(fs_dax_set_holder); Small style issue, I'd prefer a pair of functions: fs_dax_register_holder(struct dax_device *dax_dev, void *holder, const struct dax_holder_operations *ops) fs_dax_unregister_holder(struct dax_device *dax_dev) ...rather than open coding unregister as a special set that passes NULL arguments. > +void *fs_dax_get_holder(struct dax_device *dax_dev) > +{ > + return dax_get_holder(dax_dev); Does dax_get_holder() have a lockdep_assert to check that the caller has at least a read_lock? Please add kernel-doc for this api to indicate the locking context expectations. The rest of this looks plausibly ok to me, but it would be up to xfs folks to comment on the details. I'm not entirely comfortable with these handlers assuming DAX, i.e. they should also one day be useful for page cache memory failure notifications, but that support can come later. -- dm-devel mailing list dm-devel@redhat.com https://listman.redhat.com/mailman/listinfo/dm-devel
next prev parent reply other threads:[~2021-08-20 22:59 UTC|newest] Thread overview: 90+ messages / expand[flat|nested] mbox.gz Atom feed top 2021-07-30 10:01 [PATCH RESEND v6 0/9] fsdax: introduce fs query to support reflink Shiyang Ruan 2021-07-30 10:01 ` [dm-devel] " Shiyang Ruan 2021-07-30 10:01 ` [PATCH RESEND v6 1/9] pagemap: Introduce ->memory_failure() Shiyang Ruan 2021-07-30 10:01 ` [dm-devel] " Shiyang Ruan 2021-08-06 1:17 ` Jane Chu 2021-08-06 1:17 ` [dm-devel] " Jane Chu 2021-08-16 17:20 ` Jane Chu 2021-08-16 17:20 ` [dm-devel] " Jane Chu 2021-08-17 1:44 ` ruansy.fnst 2021-08-17 1:44 ` [dm-devel] " ruansy.fnst 2021-08-18 5:43 ` Jane Chu 2021-08-18 5:43 ` [dm-devel] " Jane Chu 2021-08-18 6:08 ` Jane Chu 2021-08-18 6:08 ` [dm-devel] " Jane Chu 2021-08-18 7:52 ` ruansy.fnst 2021-08-18 7:52 ` [dm-devel] " ruansy.fnst 2021-08-18 17:10 ` Dan Williams 2021-08-18 17:10 ` [dm-devel] " Dan Williams 2021-08-18 17:10 ` Dan Williams 2021-08-23 13:21 ` hch 2021-08-23 13:21 ` [dm-devel] " hch 2021-08-18 15:52 ` Darrick J. Wong 2021-08-18 15:52 ` [dm-devel] " Darrick J. Wong 2021-08-19 7:18 ` Jane Chu 2021-08-19 7:18 ` [dm-devel] " Jane Chu 2021-08-19 8:11 ` Jane Chu 2021-08-19 8:11 ` [dm-devel] " Jane Chu 2021-08-19 9:10 ` ruansy.fnst 2021-08-19 9:10 ` [dm-devel] " ruansy.fnst 2021-08-19 20:50 ` Jane Chu 2021-08-19 20:50 ` [dm-devel] " Jane Chu 2021-08-20 16:07 ` Dan Williams 2021-08-20 16:07 ` Dan Williams 2021-08-20 16:07 ` [dm-devel] " Dan Williams 2021-07-30 10:01 ` [PATCH RESEND v6 2/9] dax: Introduce holder for dax_device Shiyang Ruan 2021-07-30 10:01 ` [dm-devel] " Shiyang Ruan 2021-08-06 1:02 ` Jane Chu 2021-08-06 1:02 ` [dm-devel] " Jane Chu 2021-08-17 1:45 ` ruansy.fnst 2021-08-17 1:45 ` [dm-devel] " ruansy.fnst 2021-08-20 16:06 ` Dan Williams 2021-08-20 16:06 ` Dan Williams 2021-08-20 16:06 ` [dm-devel] " Dan Williams 2021-08-20 20:19 ` Dan Williams 2021-08-20 20:19 ` [dm-devel] " Dan Williams 2021-08-20 20:19 ` Dan Williams 2021-07-30 10:01 ` [PATCH RESEND v6 3/9] mm: factor helpers for memory_failure_dev_pagemap Shiyang Ruan 2021-07-30 10:01 ` [dm-devel] " Shiyang Ruan 2021-08-06 1:00 ` Jane Chu 2021-08-06 1:00 ` [dm-devel] " Jane Chu 2021-08-20 16:54 ` Dan Williams 2021-08-20 16:54 ` [dm-devel] " Dan Williams 2021-08-20 16:54 ` Dan Williams 2021-07-30 10:01 ` [PATCH RESEND v6 4/9] pmem,mm: Implement ->memory_failure in pmem driver Shiyang Ruan 2021-07-30 10:01 ` [dm-devel] [PATCH RESEND v6 4/9] pmem, mm: " Shiyang Ruan 2021-08-20 20:51 ` [PATCH RESEND v6 4/9] pmem,mm: " Dan Williams 2021-08-20 20:51 ` [dm-devel] [PATCH RESEND v6 4/9] pmem, mm: " Dan Williams 2021-08-20 20:51 ` [PATCH RESEND v6 4/9] pmem,mm: " Dan Williams 2021-07-30 10:01 ` [PATCH RESEND v6 5/9] mm: Introduce mf_dax_kill_procs() for fsdax case Shiyang Ruan 2021-07-30 10:01 ` [dm-devel] " Shiyang Ruan 2021-08-06 0:59 ` Jane Chu 2021-08-06 0:59 ` [dm-devel] " Jane Chu 2021-08-20 22:40 ` Dan Williams 2021-08-20 22:40 ` [dm-devel] " Dan Williams 2021-08-20 22:40 ` Dan Williams 2021-07-30 10:01 ` [PATCH RESEND v6 6/9] xfs: Implement ->notify_failure() for XFS Shiyang Ruan 2021-07-30 10:01 ` [dm-devel] " Shiyang Ruan 2021-08-06 0:50 ` Jane Chu 2021-08-06 0:50 ` [dm-devel] " Jane Chu 2021-08-20 22:56 ` Dan Williams 2021-08-20 22:56 ` [dm-devel] " Dan Williams 2021-08-20 22:56 ` Dan Williams 2021-08-20 22:59 ` Dan Williams [this message] 2021-08-20 22:59 ` [dm-devel] " Dan Williams 2021-08-20 22:59 ` Dan Williams 2021-07-30 10:01 ` [PATCH RESEND v6 7/9] dm: Introduce ->rmap() to find bdev offset Shiyang Ruan 2021-07-30 10:01 ` [dm-devel] " Shiyang Ruan 2021-08-20 23:46 ` Dan Williams 2021-08-20 23:46 ` [dm-devel] " Dan Williams 2021-08-20 23:46 ` Dan Williams 2021-07-30 10:01 ` [PATCH RESEND v6 8/9] md: Implement dax_holder_operations Shiyang Ruan 2021-07-30 10:01 ` [dm-devel] " Shiyang Ruan 2021-08-06 0:48 ` Jane Chu 2021-08-06 0:48 ` [dm-devel] " Jane Chu 2021-08-17 1:59 ` ruansy.fnst 2021-08-17 1:59 ` [dm-devel] " ruansy.fnst 2021-07-30 10:01 ` [PATCH RESEND v6 9/9] fsdax: add exception for reflinked files Shiyang Ruan 2021-07-30 10:01 ` [dm-devel] " Shiyang Ruan 2021-08-06 0:46 ` Jane Chu 2021-08-06 0:46 ` [dm-devel] " Jane Chu
Reply instructions: You may reply publicly to this message via plain-text email using any one of the following methods: * Save the following mbox file, import it into your mail client, and reply-to-all from there: mbox Avoid top-posting and favor interleaved quoting: https://en.wikipedia.org/wiki/Posting_style#Interleaved_style * Reply using the --to, --cc, and --in-reply-to switches of git-send-email(1): git send-email \ --in-reply-to='CAPcyv4h8eUKYDz+KLzXeMTEKc03k=8juXtYjYj+XSVQ5ww=KyQ@mail.gmail.com' \ --to=dan.j.williams@intel.com \ --cc=agk@redhat.com \ --cc=david@fromorbit.com \ --cc=djwong@kernel.org \ --cc=dm-devel@redhat.com \ --cc=hch@lst.de \ --cc=linux-fsdevel@vger.kernel.org \ --cc=linux-kernel@vger.kernel.org \ --cc=linux-mm@kvack.org \ --cc=linux-xfs@vger.kernel.org \ --cc=nvdimm@lists.linux.dev \ --cc=ruansy.fnst@fujitsu.com \ --cc=snitzer@redhat.com \ /path/to/YOUR_REPLY https://kernel.org/pub/software/scm/git/docs/git-send-email.html * If your mail client supports setting the In-Reply-To header via mailto: links, try the mailto: linkBe sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.