Re: [PATCH v3 12/13] dax: handle truncate of dma-busy pages
On Fri, Oct 20, 2017 at 10:27:22AM -0700, Dan Williams wrote: > On Fri, Oct 20, 2017 at 9:32 AM, Christoph Hellwig wrote: > > On Fri, Oct 20, 2017 at 08:42:00AM -0700, Dan Williams wrote: > >> I agree, but it needs quite a bit more thought and restructuring of > >> the truncate path. I also wonder how we reclaim those stranded > >> filesystem blocks, but a first approximation is wait for the > >> administrator to delete them or auto-delete them at the next mount. > >> XFS seems well prepared to reflink-swap these DMA blocks around, but > >> I'm not sure about EXT4. > > > > reflink still is an optional and experimental feature in XFS. That > > being said we should not need to swap block pointers around on disk. > > We just need to prevent the block allocator from reusing the blocks > > for new allocations, and we have code for that, both for transactions > > that haven't been committed to disk yet, and for deleted blocks > > undergoing discard operations. > > > > But as mentioned in my second mail from this morning I'm not even > > sure we need that. For short-term elevated page counts like normal > > get_user_pages users I think we can just wait for the page count > > to reach zero, while for abuses of get_user_pages for long term > > pinning memory (not sure if anyone but rdma is doing that) we'll need > > something like FL_LAYOUT leases to release the mapping. > > I'll take a look at hooking this up through a page-idle callback. Can > I get some breadcrumbs to grep for from XFS folks on how to set/clear > the busy state of extents? See fs/xfs/xfs_extent_busy.c. Brian ___ Linux-nvdimm mailing list Linux-nvdimm@lists.01.org https://lists.01.org/mailman/listinfo/linux-nvdimm
Re: [PATCH 14/23] xfs: use the common helper uuid_is_null()
On Thu, May 18, 2017 at 08:26:56AM +0200, Christoph Hellwig wrote:
> From: Amir Goldstein
>
> Use the common helper uuid_is_null() and remove the xfs specific
> helper uuid_is_nil().
>
> The common helper does not check for the NULL pointer value as
> xfs helper did, but xfs code never calls the helper with a pointer
> that can be NULL.
>
> Conform comments and warning strings to use the term 'null uuid'
> instead of 'nil uuid', because this is the terminology used by
> lib/uuid.c and its users. It is also the terminology used in
> userspace by libuuid and xfsprogs.
>
> Signed-off-by: Amir Goldstein
> [hch: remove now unused uuid.[ch]]
> Signed-off-by: Christoph Hellwig
> ---
Reviewed-by: Brian Foster
> fs/xfs/Makefile | 3 +--
> fs/xfs/uuid.c| 32
> fs/xfs/uuid.h| 23 ---
> fs/xfs/xfs_linux.h | 1 -
> fs/xfs/xfs_log_recover.c | 6 +++---
> fs/xfs/xfs_mount.c | 8
> 6 files changed, 8 insertions(+), 65 deletions(-)
> delete mode 100644 fs/xfs/uuid.c
> delete mode 100644 fs/xfs/uuid.h
>
> diff --git a/fs/xfs/Makefile b/fs/xfs/Makefile
> index 5c90f82b8f6b..a6e955bfead8 100644
> --- a/fs/xfs/Makefile
> +++ b/fs/xfs/Makefile
> @@ -98,8 +98,7 @@ xfs-y += xfs_aops.o \
> xfs_sysfs.o \
> xfs_trans.o \
> xfs_xattr.o \
> -kmem.o \
> -uuid.o
> +kmem.o
>
> # low-level transaction/log code
> xfs-y+= xfs_log.o \
> diff --git a/fs/xfs/uuid.c b/fs/xfs/uuid.c
> deleted file mode 100644
> index 737c186ea98b..
> --- a/fs/xfs/uuid.c
> +++ /dev/null
> @@ -1,32 +0,0 @@
> -/*
> - * Copyright (c) 2000-2003,2005 Silicon Graphics, Inc.
> - * All Rights Reserved.
> - *
> - * This program is free software; you can redistribute it and/or
> - * modify it under the terms of the GNU General Public License as
> - * published by the Free Software Foundation.
> - *
> - * This program is distributed in the hope that it would be useful,
> - * but WITHOUT ANY WARRANTY; without even the implied warranty of
> - * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
> - * GNU General Public License for more details.
> - *
> - * You should have received a copy of the GNU General Public License
> - * along with this program; if not, write the Free Software Foundation,
> - * Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301 USA
> - */
> -#include
> -
> -int
> -uuid_is_nil(uuid_t *uuid)
> -{
> - int i;
> - char*cp = (char *)uuid;
> -
> - if (uuid == NULL)
> - return 0;
> - /* implied check of version number here... */
> - for (i = 0; i < sizeof *uuid; i++)
> - if (*cp++) return 0;/* not nil */
> - return 1; /* is nil */
> -}
> diff --git a/fs/xfs/uuid.h b/fs/xfs/uuid.h
> deleted file mode 100644
> index 5aea49bf0963..
> --- a/fs/xfs/uuid.h
> +++ /dev/null
> @@ -1,23 +0,0 @@
> -/*
> - * Copyright (c) 2000-2003,2005 Silicon Graphics, Inc.
> - * All Rights Reserved.
> - *
> - * This program is free software; you can redistribute it and/or
> - * modify it under the terms of the GNU General Public License as
> - * published by the Free Software Foundation.
> - *
> - * This program is distributed in the hope that it would be useful,
> - * but WITHOUT ANY WARRANTY; without even the implied warranty of
> - * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
> - * GNU General Public License for more details.
> - *
> - * You should have received a copy of the GNU General Public License
> - * along with this program; if not, write the Free Software Foundation,
> - * Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301 USA
> - */
> -#ifndef __XFS_SUPPORT_UUID_H__
> -#define __XFS_SUPPORT_UUID_H__
> -
> -extern int uuid_is_nil(uuid_t *uuid);
> -
> -#endif /* __XFS_SUPPORT_UUID_H__ */
> diff --git a/fs/xfs/xfs_linux.h b/fs/xfs/xfs_linux.h
> index 2c33d915e550..2d167fe643ec 100644
> --- a/fs/xfs/xfs_linux.h
> +++ b/fs/xfs/xfs_linux.h
> @@ -43,7 +43,6 @@ typedef __u32 xfs_nlink_t;
>
> #include "kmem.h"
> #include "mrlock.h"
> -#include "uuid.h"
>
> #include
> #include
> diff --git a/fs/xfs/xfs_log_recover.c b/fs/xfs/xfs_log_recover.c
> index cd0b077deb35..8cec1e5505a4 100644
> --- a/fs/xfs/xfs_log_r
Re: [PATCH 12/23] xfs: remove uuid_getnodeuniq and xfs_uu_t
On Thu, May 18, 2017 at 08:26:54AM +0200, Christoph Hellwig wrote:
> Directly use the v1 intepretation of uuid_t instead.
>
> Signed-off-by: Christoph Hellwig
> ---
Reviewed-by: Brian Foster
> fs/xfs/uuid.c | 25 -
> fs/xfs/uuid.h | 1 -
> fs/xfs/xfs_mount.c | 6 +-
> 3 files changed, 5 insertions(+), 27 deletions(-)
>
> diff --git a/fs/xfs/uuid.c b/fs/xfs/uuid.c
> index 29ed78c8637b..737c186ea98b 100644
> --- a/fs/xfs/uuid.c
> +++ b/fs/xfs/uuid.c
> @@ -17,31 +17,6 @@
> */
> #include
>
> -/* IRIX interpretation of an uuid_t */
> -typedef struct {
> - __be32 uu_timelow;
> - __be16 uu_timemid;
> - __be16 uu_timehi;
> - __be16 uu_clockseq;
> - __be16 uu_node[3];
> -} xfs_uu_t;
> -
> -/*
> - * uuid_getnodeuniq - obtain the node unique fields of a UUID.
> - *
> - * This is not in any way a standard or condoned UUID function;
> - * it just something that's needed for user-level file handles.
> - */
> -void
> -uuid_getnodeuniq(uuid_t *uuid, int fsid [2])
> -{
> - xfs_uu_t *uup = (xfs_uu_t *)uuid;
> -
> - fsid[0] = (be16_to_cpu(uup->uu_clockseq) << 16) |
> -be16_to_cpu(uup->uu_timemid);
> - fsid[1] = be32_to_cpu(uup->uu_timelow);
> -}
> -
> int
> uuid_is_nil(uuid_t *uuid)
> {
> diff --git a/fs/xfs/uuid.h b/fs/xfs/uuid.h
> index 86bbed071e79..5aea49bf0963 100644
> --- a/fs/xfs/uuid.h
> +++ b/fs/xfs/uuid.h
> @@ -19,6 +19,5 @@
> #define __XFS_SUPPORT_UUID_H__
>
> extern int uuid_is_nil(uuid_t *uuid);
> -extern void uuid_getnodeuniq(uuid_t *uuid, int fsid [2]);
>
> #endif /* __XFS_SUPPORT_UUID_H__ */
> diff --git a/fs/xfs/xfs_mount.c b/fs/xfs/xfs_mount.c
> index 2eaf81859166..742e4a61c0bc 100644
> --- a/fs/xfs/xfs_mount.c
> +++ b/fs/xfs/xfs_mount.c
> @@ -793,7 +793,11 @@ xfs_mountfs(
>* Copies the low order bits of the timestamp and the randomly
>* set "sequence" number out of a UUID.
>*/
> - uuid_getnodeuniq(&sbp->sb_uuid, mp->m_fixedfsid);
> + mp->m_fixedfsid[0] =
> + ((u32)sbp->sb_uuid.v1.clock_seq_hi_and_reserved << 24) |
> + ((u32)sbp->sb_uuid.v1.clock_seq_low << 16) |
> + be16_to_cpu(sbp->sb_uuid.v1.time_mid);
> + mp->m_fixedfsid[1] = be32_to_cpu(sbp->sb_uuid.v1.time_low);
>
> mp->m_dmevmask = 0; /* not persistent; set after each mount */
>
> --
> 2.11.0
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-xfs" in
> the body of a message to majord...@vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
___
Linux-nvdimm mailing list
Linux-nvdimm@lists.01.org
https://lists.01.org/mailman/listinfo/linux-nvdimm
Re: [PATCH 02/23] xfs: use uuid_be to implement the uuid_t type
On Thu, May 18, 2017 at 08:26:44AM +0200, Christoph Hellwig wrote:
> Use the generic Linux definition to implement our UUID type, this will
> allow using more generic infrastructure in the future.
>
> Signed-off-by: Christoph Hellwig
> ---
Reviewed-by: Brian Foster
> fs/xfs/uuid.h | 4
> fs/xfs/xfs_linux.h | 3 +++
> 2 files changed, 3 insertions(+), 4 deletions(-)
>
> diff --git a/fs/xfs/uuid.h b/fs/xfs/uuid.h
> index 104db0f3bed6..4f1441ba4fa5 100644
> --- a/fs/xfs/uuid.h
> +++ b/fs/xfs/uuid.h
> @@ -18,10 +18,6 @@
> #ifndef __XFS_SUPPORT_UUID_H__
> #define __XFS_SUPPORT_UUID_H__
>
> -typedef struct {
> - unsigned char __u_bits[16];
> -} uuid_t;
> -
> extern int uuid_is_nil(uuid_t *uuid);
> extern int uuid_equal(uuid_t *uuid1, uuid_t *uuid2);
> extern void uuid_getnodeuniq(uuid_t *uuid, int fsid [2]);
> diff --git a/fs/xfs/xfs_linux.h b/fs/xfs/xfs_linux.h
> index 044fb0e15390..89ee5ec66837 100644
> --- a/fs/xfs/xfs_linux.h
> +++ b/fs/xfs/xfs_linux.h
> @@ -19,6 +19,7 @@
> #define __XFS_LINUX__
>
> #include
> +#include
>
> /*
> * Kernel specific type declarations for XFS
> @@ -38,6 +39,8 @@ typedef __s64 xfs_daddr_t;/*
> type */
> typedef __u32xfs_dev_t;
> typedef __u32xfs_nlink_t;
>
> +typedef uuid_be uuid_t;
> +
> #include "xfs_types.h"
>
> #include "kmem.h"
> --
> 2.11.0
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-xfs" in
> the body of a message to majord...@vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
___
Linux-nvdimm mailing list
Linux-nvdimm@lists.01.org
https://lists.01.org/mailman/listinfo/linux-nvdimm
Re: [PATCH 01/23] xfs: use uuid_copy() helper to abstract uuid_t
On Thu, May 18, 2017 at 08:26:43AM +0200, Christoph Hellwig wrote: > From: Amir Goldstein > > uuid_t definition is about to change. > > Signed-off-by: Amir Goldstein > Signed-off-by: Christoph Hellwig > --- Reviewed-by: Brian Foster > fs/xfs/xfs_inode_item.c | 8 ++-- > 1 file changed, 2 insertions(+), 6 deletions(-) > > diff --git a/fs/xfs/xfs_inode_item.c b/fs/xfs/xfs_inode_item.c > index 08cb7d1a4a3a..013cc78d7daf 100644 > --- a/fs/xfs/xfs_inode_item.c > +++ b/fs/xfs/xfs_inode_item.c > @@ -834,9 +834,7 @@ xfs_inode_item_format_convert( > in_f->ilf_dsize = in_f32->ilf_dsize; > in_f->ilf_ino = in_f32->ilf_ino; > /* copy biggest field of ilf_u */ > - memcpy(in_f->ilf_u.ilfu_uuid.__u_bits, > -in_f32->ilf_u.ilfu_uuid.__u_bits, > -sizeof(uuid_t)); > + uuid_copy(&in_f->ilf_u.ilfu_uuid, &in_f32->ilf_u.ilfu_uuid); > in_f->ilf_blkno = in_f32->ilf_blkno; > in_f->ilf_len = in_f32->ilf_len; > in_f->ilf_boffset = in_f32->ilf_boffset; > @@ -851,9 +849,7 @@ xfs_inode_item_format_convert( > in_f->ilf_dsize = in_f64->ilf_dsize; > in_f->ilf_ino = in_f64->ilf_ino; > /* copy biggest field of ilf_u */ > - memcpy(in_f->ilf_u.ilfu_uuid.__u_bits, > -in_f64->ilf_u.ilfu_uuid.__u_bits, > -sizeof(uuid_t)); > + uuid_copy(&in_f->ilf_u.ilfu_uuid, &in_f64->ilf_u.ilfu_uuid); > in_f->ilf_blkno = in_f64->ilf_blkno; > in_f->ilf_len = in_f64->ilf_len; > in_f->ilf_boffset = in_f64->ilf_boffset; > -- > 2.11.0 > > -- > To unsubscribe from this list: send the line "unsubscribe linux-xfs" in > the body of a message to majord...@vger.kernel.org > More majordomo info at http://vger.kernel.org/majordomo-info.html ___ Linux-nvdimm mailing list Linux-nvdimm@lists.01.org https://lists.01.org/mailman/listinfo/linux-nvdimm
Re: WARN_ON_ONCE() during generic/270
On Wed, Feb 08, 2017 at 11:23:12AM -0800, Darrick J. Wong wrote:
> > To: "Darrick J. Wong" , x...@oss.sgi.com
>
> [list moved; cc'ing linux-...@vger.kernel.org]
>
> On Wed, Feb 08, 2017 at 12:11:56PM -0700, Ross Zwisler wrote:
> > I hit the following WARN_ON_ONCE() during generic/270 with xfs (passed
> > through
> > kasan_symbolize.py):
> >
> > run fstests generic/270 at 2017-02-08 10:56:07
> > XFS (pmem0p2): Unmounting Filesystem
> > XFS (pmem0p2): DAX enabled. Warning: EXPERIMENTAL, use at your own risk
> > XFS (pmem0p2): Mounting V5 Filesystem
> > XFS (pmem0p2): Ending clean mount
> > XFS (pmem0p2): Quotacheck needed: Please wait.
> > XFS (pmem0p2): Quotacheck: Done.
> > XFS (pmem0p2): xlog_verify_grant_tail: space > BBTOB(tail_blocks)
>
> What does xfs_info say about this fs? I guess we're not trying to use
> rmap here, so there's less metadata to shove through the log.
>
> I'm also wondering which file are we trying to insert-range on, and
> what its bmap looks like?
>
I reproduced this with a generic ramdisk (no DAX) and managed to get an
ino via a tracepoint and a resulting bmap:
# xfs_io -c "fiemap -v" /mnt/fsstress.19801/p6f/d3/d3be/d4f9/f684
/mnt/fsstress.19801/p6f/d3/d3be/d4f9/f684:
EXT: FILE-OFFSET BLOCK-RANGE TOTAL FLAGS
0: [0..239]:hole 240
1: [240..391]: 794920..795071 152 0x0
2: [392..871]: hole 480
3: [872..903]: 795072..795103 32 0x0
4: [904..975]: 795104..795175 72 0x1
5: [976..1780]: hole 805
Note that this the post-shift bmap. The warning is complaining that
extents 3 and 4 are contiguous and thus can be merged. This is
unexpected on "insert space" (right shift) because, IIRC, we aren't
creating any new extent boundaries that didn't previously exist (as
opposed to collapse, which starts by punching a hole and connecting
bordering extents).
This isn't really a functional problem, more flagging an
unexpected/non-optimal but presumably still correct state. It may be the
case that the assumption of never having separate but contiguous extents
is not valid and the fix is to simply kill off the warning (perhaps
being at or near ENOSPC is a factor here). It's probably worth some
investigation to determine why this occurs for sure, however, just in
case we have a problem somewhere else...
Brian
> --D
>
> > [ cut here ]
> > WARNING: CPU: 7 PID: 23652 at fs/xfs/libxfs/xfs_bmap.c:5981[< none
> >>] xfs_bmse_shift_one+0x3da/0x4c0 fs/xfs/libxfs/xfs_bmap.c:5981
> > Modules linked in: dax_pmem nd_pmem dax nd_btt nd_e820 libnvdimm
> > CPU: 4 PID: 23652 Comm: 23288.fsstress. Not tainted
> > 4.10.0-rc7-00065-g926af627 #1
> > Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS 1.9.1-1.fc24
> > 04/01/2014
> > Call Trace:
> > [< inline >] __dump_stack lib/dump_stack.c:15
> > [< none >] dump_stack+0x86/0xc3 lib/dump_stack.c:51
> > [< none >] __warn+0xcb/0xf0 kernel/panic.c:547
> > [< none >] warn_slowpath_null+0x1d/0x20 kernel/panic.c:582
> > [< none >] xfs_bmse_shift_one+0x3da/0x4c0
> > fs/xfs/libxfs/xfs_bmap.c:5981
> > [< none >] xfs_bmap_shift_extents+0x305/0x490
> > fs/xfs/libxfs/xfs_bmap.c:6144
> > [< none >] xfs_shift_file_space+0x25f/0x320
> > fs/xfs/xfs_bmap_util.c:1475
> > [< none >] xfs_insert_file_space+0x5a/0x180
> > fs/xfs/xfs_bmap_util.c:1548
> > [< none >] xfs_file_fallocate+0x34c/0x3b0 fs/xfs/xfs_file.c:844
> >?[< none >] rcu_sync_lockdep_assert+0x2f/0x60
> > kernel/rcu/sync.c:68
> > [< none >] vfs_fallocate+0x15a/0x230 fs/open.c:320
> > [< inline >] SYSC_fallocate fs/open.c:343
> > [< none >] SyS_fallocate+0x48/0x80 fs/open.c:337
> > [< none >] entry_SYSCALL_64_fastpath+0x1f/0xc2
> > /home/rzwisler/project/linux/arch/x86/entry/entry_64.S:204
> > RIP: 0033:0x7f34dc4ff0ca
> > RSP: 002b:7ffcffa58058 EFLAGS: 0246 ORIG_RAX: 011d
> > RAX: ffda RBX: 0166 RCX: 7f34dc4ff0ca
> > RDX: 000ba000 RSI: 0020 RDI: 0003
> > RBP: 0003 R08: 007b R09: 7ffcffa5807c
> > R10: 000bc000 R11: 0246 R12: 7f34d8000de0
> > R13: R14: af4a R15:
> > ---[ end trace e24f5d4cbfc216f6 ]---
> >
> > This trace is with the current linux/master:
> >
> > commit 926af6273fc6 ("Merge
> > git://git.kernel.org/pub/scm/linux/kernel/git/davem/net")
> >
> > though I initially his this issue with a v4.9 kernel. My test setup is a
> > pair
> > of PMEM ramdisks made with the memmap command line parameter, and my
> > test filesystem is mounted with DAX.
> >
> > This can be reproduced pretty easily by just running generic/270 in a
> > loop.
> >
> > Thanks,
> > - Ross
> -
