* [PATCH v2] xfs: deprecate BMV_IF_NO_DMAPI_READ flag
@ 2021-03-31 16:26 Anthony Iliopoulos
2021-03-31 16:31 ` Darrick J. Wong
0 siblings, 1 reply; 3+ messages in thread
From: Anthony Iliopoulos @ 2021-03-31 16:26 UTC (permalink / raw)
To: linux-xfs
Use of the flag has had no effect since kernel commit 288699fecaff
("xfs: drop dmapi hooks"), which removed all dmapi related code, so
deprecate it.
Signed-off-by: Anthony Iliopoulos <ailiop@suse.com>
---
changes since v1:
- retain flag definition to prevent reuse and not break kabi, per
Darrick's suggestion.
fs/xfs/libxfs/xfs_fs.h | 4 ++--
fs/xfs/xfs_ioctl.c | 2 --
2 files changed, 2 insertions(+), 4 deletions(-)
diff --git a/fs/xfs/libxfs/xfs_fs.h b/fs/xfs/libxfs/xfs_fs.h
index 6fad140d4c8e..4ef813e00e9e 100644
--- a/fs/xfs/libxfs/xfs_fs.h
+++ b/fs/xfs/libxfs/xfs_fs.h
@@ -65,13 +65,13 @@ struct getbmapx {
/* bmv_iflags values - set by XFS_IOC_GETBMAPX caller. */
#define BMV_IF_ATTRFORK 0x1 /* return attr fork rather than data */
-#define BMV_IF_NO_DMAPI_READ 0x2 /* Do not generate DMAPI read event */
+#define BMV_IF_NO_DMAPI_READ 0x2 /* Deprecated */
#define BMV_IF_PREALLOC 0x4 /* rtn status BMV_OF_PREALLOC if req */
#define BMV_IF_DELALLOC 0x8 /* rtn status BMV_OF_DELALLOC if req */
#define BMV_IF_NO_HOLES 0x10 /* Do not return holes */
#define BMV_IF_COWFORK 0x20 /* return CoW fork rather than data */
#define BMV_IF_VALID \
- (BMV_IF_ATTRFORK|BMV_IF_NO_DMAPI_READ|BMV_IF_PREALLOC| \
+ (BMV_IF_ATTRFORK|BMV_IF_PREALLOC| \
BMV_IF_DELALLOC|BMV_IF_NO_HOLES|BMV_IF_COWFORK)
/* bmv_oflags values - returned for each non-header segment */
diff --git a/fs/xfs/xfs_ioctl.c b/fs/xfs/xfs_ioctl.c
index 99dfe89a8d08..9d3f72ef1efe 100644
--- a/fs/xfs/xfs_ioctl.c
+++ b/fs/xfs/xfs_ioctl.c
@@ -1669,8 +1669,6 @@ xfs_ioc_getbmap(
bmx.bmv_iflags = BMV_IF_ATTRFORK;
/*FALLTHRU*/
case XFS_IOC_GETBMAP:
- if (file->f_mode & FMODE_NOCMTIME)
- bmx.bmv_iflags |= BMV_IF_NO_DMAPI_READ;
/* struct getbmap is a strict subset of struct getbmapx. */
recsize = sizeof(struct getbmap);
break;
--
2.31.0
^ permalink raw reply related [flat|nested] 3+ messages in thread
* Re: [PATCH v2] xfs: deprecate BMV_IF_NO_DMAPI_READ flag
2021-03-31 16:26 [PATCH v2] xfs: deprecate BMV_IF_NO_DMAPI_READ flag Anthony Iliopoulos
@ 2021-03-31 16:31 ` Darrick J. Wong
2021-03-31 17:06 ` Anthony Iliopoulos
0 siblings, 1 reply; 3+ messages in thread
From: Darrick J. Wong @ 2021-03-31 16:31 UTC (permalink / raw)
To: Anthony Iliopoulos; +Cc: linux-xfs
On Wed, Mar 31, 2021 at 06:26:16PM +0200, Anthony Iliopoulos wrote:
> Use of the flag has had no effect since kernel commit 288699fecaff
> ("xfs: drop dmapi hooks"), which removed all dmapi related code, so
> deprecate it.
>
> Signed-off-by: Anthony Iliopoulos <ailiop@suse.com>
> ---
> changes since v1:
> - retain flag definition to prevent reuse and not break kabi, per
> Darrick's suggestion.
>
> fs/xfs/libxfs/xfs_fs.h | 4 ++--
> fs/xfs/xfs_ioctl.c | 2 --
> 2 files changed, 2 insertions(+), 4 deletions(-)
>
> diff --git a/fs/xfs/libxfs/xfs_fs.h b/fs/xfs/libxfs/xfs_fs.h
> index 6fad140d4c8e..4ef813e00e9e 100644
> --- a/fs/xfs/libxfs/xfs_fs.h
> +++ b/fs/xfs/libxfs/xfs_fs.h
> @@ -65,13 +65,13 @@ struct getbmapx {
>
> /* bmv_iflags values - set by XFS_IOC_GETBMAPX caller. */
> #define BMV_IF_ATTRFORK 0x1 /* return attr fork rather than data */
> -#define BMV_IF_NO_DMAPI_READ 0x2 /* Do not generate DMAPI read event */
> +#define BMV_IF_NO_DMAPI_READ 0x2 /* Deprecated */
> #define BMV_IF_PREALLOC 0x4 /* rtn status BMV_OF_PREALLOC if req */
> #define BMV_IF_DELALLOC 0x8 /* rtn status BMV_OF_DELALLOC if req */
> #define BMV_IF_NO_HOLES 0x10 /* Do not return holes */
> #define BMV_IF_COWFORK 0x20 /* return CoW fork rather than data */
> #define BMV_IF_VALID \
> - (BMV_IF_ATTRFORK|BMV_IF_NO_DMAPI_READ|BMV_IF_PREALLOC| \
> + (BMV_IF_ATTRFORK|BMV_IF_PREALLOC| \
> BMV_IF_DELALLOC|BMV_IF_NO_HOLES|BMV_IF_COWFORK)
What about the xfs/296 regression that the kernel robot reported?
I /think/ that's a result of removing this flag from BMV_IF_VALID, which
is used to reject unknown input flags from the GETBMAP caller. In the
current upstream the flag is valid even if it does nothing, so we have
to preserve that behavior.
--D
>
> /* bmv_oflags values - returned for each non-header segment */
> diff --git a/fs/xfs/xfs_ioctl.c b/fs/xfs/xfs_ioctl.c
> index 99dfe89a8d08..9d3f72ef1efe 100644
> --- a/fs/xfs/xfs_ioctl.c
> +++ b/fs/xfs/xfs_ioctl.c
> @@ -1669,8 +1669,6 @@ xfs_ioc_getbmap(
> bmx.bmv_iflags = BMV_IF_ATTRFORK;
> /*FALLTHRU*/
> case XFS_IOC_GETBMAP:
> - if (file->f_mode & FMODE_NOCMTIME)
> - bmx.bmv_iflags |= BMV_IF_NO_DMAPI_READ;
> /* struct getbmap is a strict subset of struct getbmapx. */
> recsize = sizeof(struct getbmap);
> break;
> --
> 2.31.0
>
^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: [PATCH v2] xfs: deprecate BMV_IF_NO_DMAPI_READ flag
2021-03-31 16:31 ` Darrick J. Wong
@ 2021-03-31 17:06 ` Anthony Iliopoulos
0 siblings, 0 replies; 3+ messages in thread
From: Anthony Iliopoulos @ 2021-03-31 17:06 UTC (permalink / raw)
To: Darrick J. Wong; +Cc: linux-xfs
On Wed, Mar 31, 2021 at 09:31:14AM -0700, Darrick J. Wong wrote:
> On Wed, Mar 31, 2021 at 06:26:16PM +0200, Anthony Iliopoulos wrote:
> > Use of the flag has had no effect since kernel commit 288699fecaff
> > ("xfs: drop dmapi hooks"), which removed all dmapi related code, so
> > deprecate it.
> >
> > Signed-off-by: Anthony Iliopoulos <ailiop@suse.com>
> > ---
> > changes since v1:
> > - retain flag definition to prevent reuse and not break kabi, per
> > Darrick's suggestion.
> >
> > fs/xfs/libxfs/xfs_fs.h | 4 ++--
> > fs/xfs/xfs_ioctl.c | 2 --
> > 2 files changed, 2 insertions(+), 4 deletions(-)
> >
> > diff --git a/fs/xfs/libxfs/xfs_fs.h b/fs/xfs/libxfs/xfs_fs.h
> > index 6fad140d4c8e..4ef813e00e9e 100644
> > --- a/fs/xfs/libxfs/xfs_fs.h
> > +++ b/fs/xfs/libxfs/xfs_fs.h
> > @@ -65,13 +65,13 @@ struct getbmapx {
> >
> > /* bmv_iflags values - set by XFS_IOC_GETBMAPX caller. */
> > #define BMV_IF_ATTRFORK 0x1 /* return attr fork rather than data */
> > -#define BMV_IF_NO_DMAPI_READ 0x2 /* Do not generate DMAPI read event */
> > +#define BMV_IF_NO_DMAPI_READ 0x2 /* Deprecated */
> > #define BMV_IF_PREALLOC 0x4 /* rtn status BMV_OF_PREALLOC if req */
> > #define BMV_IF_DELALLOC 0x8 /* rtn status BMV_OF_DELALLOC if req */
> > #define BMV_IF_NO_HOLES 0x10 /* Do not return holes */
> > #define BMV_IF_COWFORK 0x20 /* return CoW fork rather than data */
> > #define BMV_IF_VALID \
> > - (BMV_IF_ATTRFORK|BMV_IF_NO_DMAPI_READ|BMV_IF_PREALLOC| \
> > + (BMV_IF_ATTRFORK|BMV_IF_PREALLOC| \
> > BMV_IF_DELALLOC|BMV_IF_NO_HOLES|BMV_IF_COWFORK)
>
> What about the xfs/296 regression that the kernel robot reported?
>
> I /think/ that's a result of removing this flag from BMV_IF_VALID, which
> is used to reject unknown input flags from the GETBMAP caller. In the
> current upstream the flag is valid even if it does nothing, so we have
> to preserve that behavior.
In that case then it indeed needs to stay in BMV_IF_VALID. I assume it
is not worth putting flags into a deprecation schedule, so this will
have to stay there for as long as the ioctl exists. I'll send a v3.
Regards,
Anthony
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2021-03-31 17:07 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2021-03-31 16:26 [PATCH v2] xfs: deprecate BMV_IF_NO_DMAPI_READ flag Anthony Iliopoulos
2021-03-31 16:31 ` Darrick J. Wong
2021-03-31 17:06 ` Anthony Iliopoulos
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox