| Message ID | 20240808183340.483468-2-martin.oliveira@eideticom.com (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | Enable P2PDMA in Userspace RDMA | expand |
On Thu, Aug 08, 2024 at 12:33:37PM -0600, Martin Oliveira wrote: > The next patch is going to remove .page_mkwrite from kernfs and will > WARN if an mmap implementation sets .page_mkwrite. > > In preparation for that change, and to make it consistent, add a WARN to > the ->close check. > > Co-developed-by: Logan Gunthorpe <logang@deltatee.com> > Signed-off-by: Logan Gunthorpe <logang@deltatee.com> > Signed-off-by: Martin Oliveira <martin.oliveira@eideticom.com> > --- > fs/kernfs/file.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/fs/kernfs/file.c b/fs/kernfs/file.c > index 8502ef68459b..72cc51dcf870 100644 > --- a/fs/kernfs/file.c > +++ b/fs/kernfs/file.c > @@ -479,7 +479,7 @@ static int kernfs_fop_mmap(struct file *file, struct vm_area_struct *vma) > * It is not possible to successfully wrap close. > * So error if someone is trying to use close. > */ > - if (vma->vm_ops && vma->vm_ops->close) > + if (WARN_ON_ONCE(vma->vm_ops && vma->vm_ops->close)) So you just rebooted a machine that hits this, loosing data everywhere. Not nice :(
On 2024-08-08 23:37, Greg Kroah-Hartman wrote: >> diff --git a/fs/kernfs/file.c b/fs/kernfs/file.c >> index 8502ef68459b..72cc51dcf870 100644 >> --- a/fs/kernfs/file.c >> +++ b/fs/kernfs/file.c >> @@ -479,7 +479,7 @@ static int kernfs_fop_mmap(struct file *file, struct vm_area_struct *vma) >> * It is not possible to successfully wrap close. >> * So error if someone is trying to use close. >> */ >> - if (vma->vm_ops && vma->vm_ops->close) >> + if (WARN_ON_ONCE(vma->vm_ops && vma->vm_ops->close)) > > So you just rebooted a machine that hits this, loosing data everywhere. > Not nice :( Well, apologies for that, but there's no way to know what every single machine out there is doing... However if this machine is using ->close when that's clearly marked as unsupported then shouldn't we fix that?
On Fri, Aug 09, 2024 at 09:41:48AM -0600, Martin Oliveira wrote: > On 2024-08-08 23:37, Greg Kroah-Hartman wrote: > >> diff --git a/fs/kernfs/file.c b/fs/kernfs/file.c > >> index 8502ef68459b..72cc51dcf870 100644 > >> --- a/fs/kernfs/file.c > >> +++ b/fs/kernfs/file.c > >> @@ -479,7 +479,7 @@ static int kernfs_fop_mmap(struct file *file, struct vm_area_struct *vma) > >> * It is not possible to successfully wrap close. > >> * So error if someone is trying to use close. > >> */ > >> - if (vma->vm_ops && vma->vm_ops->close) > >> + if (WARN_ON_ONCE(vma->vm_ops && vma->vm_ops->close)) > > > > So you just rebooted a machine that hits this, loosing data everywhere. > > Not nice :( > > Well, apologies for that, but there's no way to know what every single machine > out there is doing... > > However if this machine is using ->close when that's clearly marked as > unsupported then shouldn't we fix that? return an error properly?
On Fri, Aug 09, 2024 at 07:37:49AM +0200, Greg Kroah-Hartman wrote: > > * It is not possible to successfully wrap close. > > * So error if someone is trying to use close. > > */ > > - if (vma->vm_ops && vma->vm_ops->close) > > + if (WARN_ON_ONCE(vma->vm_ops && vma->vm_ops->close)) > > So you just rebooted a machine that hits this, loosing data everywhere. > Not nice :( Huh. if you are stupid enough to set panic_on_warn you get to keep the pieces. And our file systems are reliable to not use data on an unclean shutdown anyway. Pleaee stop these BS arguments.
Looks good:
Reviewed-by: Christoph Hellwig <hch@lst.de>
diff --git a/fs/kernfs/file.c b/fs/kernfs/file.c index 8502ef68459b..72cc51dcf870 100644 --- a/fs/kernfs/file.c +++ b/fs/kernfs/file.c @@ -479,7 +479,7 @@ static int kernfs_fop_mmap(struct file *file, struct vm_area_struct *vma) * It is not possible to successfully wrap close. * So error if someone is trying to use close. */ - if (vma->vm_ops && vma->vm_ops->close) + if (WARN_ON_ONCE(vma->vm_ops && vma->vm_ops->close)) goto out_put; rc = 0;