* [PATCH] libsas: add sas_unregister_devs_sas_addr in flutter case
From: Jason Yan @ 2017-03-01 1:23 UTC (permalink / raw)
To: linux-scsi, martin.petersen
Cc: axboe, linux-block, miaoxie, fangwei1, wangyijing, zhaohongjiang,
Jason Yan
In sas_rediscover_dev when we call sas_get_phy_attached_dev to find the
device is ok and when in the flutter case when we call
sas_ex_phy_discover the device is gone, the sas_addr was changed to
zero.
[300247.584696] sas: ex 500e004aaaaaaa1f phy0 originated
BROADCAST(CHANGE)
[300247.663516] sas: ex 500e004aaaaaaa1f phy00:U:0 attached:
0000000000000000 (no device)
[300247.663518] sas: ex 500e004aaaaaaa1f phy 0x0 broadcast flutter
When the device is up again, the libsas checked that the old sas_addr
zero so just add a new device.
[300247.846326] sas: Expander phy change count has changed
[300247.846418] sas: ex 500e004aaaaaaa1f phy0 originated
BROADCAST(CHANGE)
[300247.846420] sas: ex 500e004aaaaaaa1f phy0 new device attached
[300247.846519] sas: ex 500e004aaaaaaa1f phy00:U:A attached:
500e004aaaaaaa00 (stp)
[300247.875873] sas: done REVALIDATING DOMAIN on port 0, pid:12565, res
0x0
This will cause a panic when these two device were destroyed. Fix this
by call sas_unregister_devs_sas_addr in the flutter case if the sas_addr
is zero.
Signed-off-by: Jason Yan <yanaijie@huawei.com>
---
drivers/scsi/libsas/sas_expander.c | 5 +++++
1 file changed, 5 insertions(+)
diff --git a/drivers/scsi/libsas/sas_expander.c b/drivers/scsi/libsas/sas_expander.c
index 570b2cb..1a784d7 100644
--- a/drivers/scsi/libsas/sas_expander.c
+++ b/drivers/scsi/libsas/sas_expander.c
@@ -2032,6 +2032,11 @@ static int sas_rediscover_dev(struct domain_device *dev, int phy_id, bool last)
sas_ex_phy_discover(dev, phy_id);
+ if (!SAS_ADDR(phy->attached_sas_addr)) {
+ sas_unregister_devs_sas_addr(dev, phy_id, last);
+ return res;
+ }
+
if (ata_dev && phy->attached_dev_type == SAS_SATA_PENDING)
action = ", needs recovery";
SAS_DPRINTK("ex %016llx phy 0x%x broadcast flutter%s\n",
--
2.5.0
^ permalink raw reply related
* connect cmd error for nvme-rdma with eventual kernel crash
From: Parav Pandit @ 2017-03-01 0:57 UTC (permalink / raw)
To: axboe@fb.com
Cc: linux-block@vger.kernel.org, Sagi Grimberg, Christoph Hellwig,
Max Gurtovoy
Hi Jens,
With your commit 2af8cbe30531eca73c8f3ba277f155fc0020b01a in linux-block gi=
t tree,
There are two requests tables. Static and dynamic of same size.
However function blk_mq_tag_to_rq() always try to get the tag from the dyna=
mic table which doesn't seem to be always initialized.
I am running nvme-rdma initiator and it fails to find the request for the g=
iven tag when command completes.
Command triggers error recovery with "tag not found" error.
Eventually kernel is crashing in blk_mq_queue_tag_busy_iter() with NULL poi=
nter. Seems to be additional bug in error recovery.
To debug, I added initializing dynamic tags as well.
blk_mq_alloc_rqs() {
tags->static_rqs[i] =3D rq;
+ tags->rqs[i] =3D rq;
This appears to resolve the issue. But that's not the fix.
It appears to me that nvme stack is broken in certain conditions with recen=
t static and dynamic rq tables change.
Can you please confirm?
Regards,
Parav Pandit
^ permalink raw reply
* Re: [PATCH v2 04/13] md: prepare for managing resync I/O pages in clean way
From: Shaohua Li @ 2017-02-28 23:30 UTC (permalink / raw)
To: Ming Lei; +Cc: Jens Axboe, linux-raid, linux-block, Christoph Hellwig
In-Reply-To: <1488296503-4987-5-git-send-email-tom.leiming@gmail.com>
On Tue, Feb 28, 2017 at 11:41:34PM +0800, Ming Lei wrote:
> Now resync I/O use bio's bec table to manage pages,
> this way is very hacky, and may not work any more
> once multipage bvec is introduced.
>
> So introduce helpers and new data structure for
> managing resync I/O pages more cleanly.
>
> Signed-off-by: Ming Lei <tom.leiming@gmail.com>
> ---
> drivers/md/md.h | 54 ++++++++++++++++++++++++++++++++++++++++++++++++++++++
> 1 file changed, 54 insertions(+)
>
> diff --git a/drivers/md/md.h b/drivers/md/md.h
> index 1d63239a1be4..b5a638d85cb4 100644
> --- a/drivers/md/md.h
> +++ b/drivers/md/md.h
> @@ -720,4 +720,58 @@ static inline void mddev_check_writesame(struct mddev *mddev, struct bio *bio)
> #define RESYNC_BLOCK_SIZE (64*1024)
> #define RESYNC_PAGES ((RESYNC_BLOCK_SIZE + PAGE_SIZE-1) / PAGE_SIZE)
>
> +/* for managing resync I/O pages */
> +struct resync_pages {
> + unsigned idx; /* for get/put page from the pool */
> + void *raid_bio;
> + struct page *pages[RESYNC_PAGES];
> +};
I'd like this to be embedded into r1bio directly. Not sure if we really need a
structure.
> +
> +static inline int resync_alloc_pages(struct resync_pages *rp,
> + gfp_t gfp_flags)
> +{
> + int i;
> +
> + for (i = 0; i < RESYNC_PAGES; i++) {
> + rp->pages[i] = alloc_page(gfp_flags);
> + if (!rp->pages[i])
> + goto out_free;
> + }
> +
> + return 0;
> +
> + out_free:
> + while (--i >= 0)
> + __free_page(rp->pages[i]);
> + return -ENOMEM;
> +}
> +
> +static inline void resync_free_pages(struct resync_pages *rp)
> +{
> + int i;
> +
> + for (i = 0; i < RESYNC_PAGES; i++)
> + __free_page(rp->pages[i]);
Since we will use get_page, shouldn't this be put_page?
> +}
> +
> +static inline void resync_get_all_pages(struct resync_pages *rp)
> +{
> + int i;
> +
> + for (i = 0; i < RESYNC_PAGES; i++)
> + get_page(rp->pages[i]);
> +}
> +
> +static inline struct page *resync_fetch_page(struct resync_pages *rp)
> +{
> + if (WARN_ON_ONCE(rp->idx >= RESYNC_PAGES))
> + return NULL;
> + return rp->pages[rp->idx++];
I'd like the caller explicitly specify the index instead of a global variable
to track it, which will make the code more understandable and less error prone.
> +}
> +
> +static inline bool resync_page_available(struct resync_pages *rp)
> +{
> + return rp->idx < RESYNC_PAGES;
> +}
Then we don't need this obscure API.
Thanks,
Shaohua
^ permalink raw reply
* Re: [PATCH 07/12] xfs: implement failure-atomic writes
From: Darrick J. Wong @ 2017-02-28 23:09 UTC (permalink / raw)
To: Christoph Hellwig; +Cc: linux-fsdevel, linux-xfs, linux-block
In-Reply-To: <20170228145737.19016-8-hch@lst.de>
On Tue, Feb 28, 2017 at 06:57:32AM -0800, Christoph Hellwig wrote:
> If O_ATOMIC is specified in the open flags this will cause XFS to
> allocate new extents in the COW for even if overwriting existing data,
"COW fork" ^^^^^^^
The previous patch's commit message also has that quirk.
> and not remap them into the data fork until ->fsync is called,
> at which point the whole range will be atomically remapped into the
> data fork. This allows applications to ѕafely overwrite data instead
> of having to do double writes.
By the way, the copy on write code remembers the extents it has
allocated for CoW staging in the refcount btree so that it can free them
after a crash, which means that O_ATOMIC requires reflink to be enabled.
There doesn't seem to be any explicit checking that reflink is even
enabled, which will probably just lead to weird crashes on a pre-reflink
xfs.
FWIW I didn't see any checking anywhere (vfs or xfs) that the filesystem
can actually support O_ATOMIC. If the FS doesn't support atomic writes,
shouldn't the kernel send EINVAL or something back to userspace?
> Signed-off-by: Christoph Hellwig <hch@lst.de>
> ---
> fs/xfs/xfs_aops.c | 18 +++++++++-----
> fs/xfs/xfs_aops.h | 4 ++-
> fs/xfs/xfs_file.c | 17 +++++++++++++
> fs/xfs/xfs_iomap.c | 18 ++++++++------
> fs/xfs/xfs_reflink.c | 69 ++++++++++++++++++++++++++++++++++------------------
> fs/xfs/xfs_reflink.h | 5 ++--
> 6 files changed, 91 insertions(+), 40 deletions(-)
>
> diff --git a/fs/xfs/xfs_aops.c b/fs/xfs/xfs_aops.c
> index c78b585b3d84..1c5efbb05b47 100644
> --- a/fs/xfs/xfs_aops.c
> +++ b/fs/xfs/xfs_aops.c
> @@ -292,6 +292,7 @@ xfs_end_io(
> if (unlikely(error)) {
> switch (ioend->io_type) {
> case XFS_IO_COW:
> + case XFS_IO_ATOMIC:
So we cancel the CoW staging blocks if the write was atomic and failed.
Later in the !error case we remap the blocks if it was a cow write, but
leave the mapping in memory if the write was atomic. That is consistent
with the commit message, good.
At the start of xfs_reflink.c is a long block comment describing how the
copy on write mechanism works. Since O_ATOMIC is a variant on CoW (it's
basically CoW with remapping deferred until fsync), please update the
comment so that the comments capture the details of how atomic writes
work.
(IOWs: Dave asked me to leave the big comment, so I'm going to try to
keep it fairly up to date.)
> xfs_reflink_cancel_cow_range(ip, offset, size, 0);
> break;
> }
> @@ -327,7 +328,9 @@ xfs_end_bio(
> struct xfs_ioend *ioend = bio->bi_private;
> struct xfs_mount *mp = XFS_I(ioend->io_inode)->i_mount;
>
> - if (ioend->io_type == XFS_IO_UNWRITTEN || ioend->io_type == XFS_IO_COW)
> + if (ioend->io_type == XFS_IO_UNWRITTEN ||
> + ioend->io_type == XFS_IO_COW ||
> + ioend->io_type == XFS_IO_ATOMIC)
> queue_work(mp->m_unwritten_workqueue, &ioend->io_work);
> else if (ioend->io_append_trans)
> queue_work(mp->m_data_workqueue, &ioend->io_work);
> @@ -354,6 +357,7 @@ xfs_map_blocks(
> return -EIO;
>
> ASSERT(type != XFS_IO_COW);
> + ASSERT(type != XFS_IO_ATOMIC);
> if (type == XFS_IO_UNWRITTEN)
> bmapi_flags |= XFS_BMAPI_IGSTATE;
>
> @@ -768,7 +772,8 @@ xfs_map_cow(
> struct xfs_writepage_ctx *wpc,
> struct inode *inode,
> loff_t offset,
> - unsigned int *new_type)
> + unsigned int *new_type,
> + bool atomic)
> {
> struct xfs_inode *ip = XFS_I(inode);
> struct xfs_bmbt_irec imap;
> @@ -778,10 +783,10 @@ xfs_map_cow(
> /*
> * If we already have a valid COW mapping keep using it.
> */
> - if (wpc->io_type == XFS_IO_COW) {
> + if (wpc->io_type == XFS_IO_COW || wpc->io_type == XFS_IO_ATOMIC) {
> wpc->imap_valid = xfs_imap_valid(inode, &wpc->imap, offset);
> if (wpc->imap_valid) {
> - *new_type = XFS_IO_COW;
> + *new_type = wpc->io_type;
> return 0;
> }
> }
> @@ -807,7 +812,7 @@ xfs_map_cow(
> return error;
> }
>
> - wpc->io_type = *new_type = XFS_IO_COW;
> + wpc->io_type = *new_type = atomic ? XFS_IO_ATOMIC : XFS_IO_COW;
> wpc->imap_valid = true;
> wpc->imap = imap;
> return 0;
> @@ -886,7 +891,8 @@ xfs_writepage_map(
> }
>
> if (XFS_I(inode)->i_cowfp) {
> - error = xfs_map_cow(wpc, inode, offset, &new_type);
> + error = xfs_map_cow(wpc, inode, offset, &new_type,
> + buffer_atomic(bh));
> if (error)
> goto out;
> }
> diff --git a/fs/xfs/xfs_aops.h b/fs/xfs/xfs_aops.h
> index cc174ec6c2fd..798e653e68b6 100644
> --- a/fs/xfs/xfs_aops.h
> +++ b/fs/xfs/xfs_aops.h
> @@ -29,6 +29,7 @@ enum {
> XFS_IO_UNWRITTEN, /* covers allocated but uninitialized data */
> XFS_IO_OVERWRITE, /* covers already allocated extent */
> XFS_IO_COW, /* covers copy-on-write extent */
> + XFS_IO_ATOMIC, /* atomic write */
> };
>
> #define XFS_IO_TYPES \
> @@ -36,7 +37,8 @@ enum {
> { XFS_IO_DELALLOC, "delalloc" }, \
> { XFS_IO_UNWRITTEN, "unwritten" }, \
> { XFS_IO_OVERWRITE, "overwrite" }, \
> - { XFS_IO_COW, "CoW" }
> + { XFS_IO_COW, "CoW" }, \
> + { XFS_IO_ATOMIC, "atomic" }
>
> /*
> * Structure for buffered I/O completions.
> diff --git a/fs/xfs/xfs_file.c b/fs/xfs/xfs_file.c
> index 086440e79b86..a7d8324b59c5 100644
> --- a/fs/xfs/xfs_file.c
> +++ b/fs/xfs/xfs_file.c
> @@ -160,6 +160,12 @@ xfs_file_fsync(
> else if (mp->m_logdev_targp != mp->m_ddev_targp)
> xfs_blkdev_issue_flush(mp->m_ddev_targp);
>
> + if (file->f_flags & O_ATOMIC) {
> + error = xfs_reflink_end_cow(ip, start, end - start + 1);
> + if (error)
> + return error;
> + }
I suppose it goes without saying that userspace will have to coordinate
its O_ATOMIC writes to the file. What if this happens?
Process A Process B
Open atomic file Open atomic file
Dirty some pages Dirty some other pages
fsync
Successful fsync return
Dirty more pages
Dirty more of some other pages
<system crash>
When we come back up, the file contents will reflect everything A wrote
up to fsync, and (since fsync flushed everything) everything B wrote in
"dirty some other pages", even though it hadn't reached fsync. Won't
this be surprising to B since it expected that disk mappings don't get
updated until it fsyncs?
Practically speaking, I wonder how often this will come up in the real
world, but it does seem to be a potential downside. Per *file tracking
sounds like a bigger bookkeeping nightmare.
> +
> /*
> * All metadata updates are logged, which means that we just have to
> * flush the log up to the latest LSN that touched the inode. If we have
> @@ -457,6 +463,9 @@ xfs_dio_write_end_io(
> }
> spin_unlock(&ip->i_flags_lock);
>
> + if (iocb->ki_filp->f_flags & O_ATOMIC)
> + return 0;
> +
> if (flags & IOMAP_DIO_COW) {
> error = xfs_reflink_end_cow(ip, offset, size);
> if (error)
> @@ -529,6 +538,12 @@ xfs_file_dio_aio_write(
> unaligned_io = 1;
>
> /*
> + * We need filesystem block alignment to provide atomic commits.
> + */
> + if (file->f_flags & O_ATOMIC)
> + return -EINVAL;
> +
> + /*
> * We can't properly handle unaligned direct I/O to reflink
> * files yet, as we can't unshare a partial block.
> */
> @@ -892,6 +907,8 @@ xfs_file_open(
> return -EFBIG;
> if (XFS_FORCED_SHUTDOWN(XFS_M(inode->i_sb)))
> return -EIO;
> + if (file->f_flags & O_ATOMIC)
> + printk_ratelimited("O_ATOMIC!\n");
Per above,
if (file->f_flags & O_ATOMIC) {
if (!xfs_sb_version_hasreflink(...))
return -EPROTONOSUPPORT;
printk_ratelimited("EXPERIMENTAL atomic writes feature in use!\n");
}
> return 0;
> }
>
> diff --git a/fs/xfs/xfs_iomap.c b/fs/xfs/xfs_iomap.c
> index 5d68b4279016..b686a6bd2db4 100644
> --- a/fs/xfs/xfs_iomap.c
> +++ b/fs/xfs/xfs_iomap.c
> @@ -559,13 +559,14 @@ xfs_file_iomap_begin_delay(
>
> eof = !xfs_iext_lookup_extent(ip, ifp, offset_fsb, &idx, &got);
> if (!eof && got.br_startoff <= offset_fsb) {
> - if (xfs_is_reflink_inode(ip)) {
> + if ((flags & IOMAP_ATOMIC) || xfs_is_reflink_inode(ip)) {
> bool shared;
>
> end_fsb = min(XFS_B_TO_FSB(mp, offset + count),
> maxbytes_fsb);
> xfs_trim_extent(&got, offset_fsb, end_fsb - offset_fsb);
> - error = xfs_reflink_reserve_cow(ip, &got, &shared);
> + error = xfs_reflink_reserve_cow(ip, &got, &shared,
> + (flags & IOMAP_ATOMIC));
> if (error)
> goto out_unlock;
> }
> @@ -951,7 +952,7 @@ static inline bool need_excl_ilock(struct xfs_inode *ip, unsigned flags)
> */
> if (xfs_is_reflink_inode(ip) && (flags & (IOMAP_WRITE | IOMAP_ZERO)))
> return true;
> - if ((flags & IOMAP_DIRECT) && (flags & IOMAP_WRITE))
> + if ((flags & (IOMAP_DIRECT | IOMAP_ATOMIC)) && (flags & IOMAP_WRITE))
> return true;
> return false;
> }
> @@ -976,7 +977,8 @@ xfs_file_iomap_begin(
> return -EIO;
>
> if (((flags & (IOMAP_WRITE | IOMAP_DIRECT)) == IOMAP_WRITE) &&
> - !IS_DAX(inode) && !xfs_get_extsz_hint(ip)) {
> + ((flags & IOMAP_ATOMIC) ||
> + (!IS_DAX(inode) && !xfs_get_extsz_hint(ip)))) {
> /* Reserve delalloc blocks for regular writeback. */
> return xfs_file_iomap_begin_delay(inode, offset, length, flags,
> iomap);
> @@ -1008,15 +1010,17 @@ xfs_file_iomap_begin(
> goto out_unlock;
> }
>
> - if ((flags & (IOMAP_WRITE | IOMAP_ZERO)) && xfs_is_reflink_inode(ip)) {
> + if ((flags & (IOMAP_WRITE | IOMAP_ZERO)) &&
> + ((flags & IOMAP_ATOMIC) || xfs_is_reflink_inode(ip))) {
> if (flags & IOMAP_DIRECT) {
> /* may drop and re-acquire the ilock */
> error = xfs_reflink_allocate_cow(ip, &imap, &shared,
> - &lockmode);
> + &lockmode, flags & IOMAP_ATOMIC);
> if (error)
> goto out_unlock;
> } else {
> - error = xfs_reflink_reserve_cow(ip, &imap, &shared);
> + error = xfs_reflink_reserve_cow(ip, &imap, &shared,
> + false);
> if (error)
> goto out_unlock;
> }
> diff --git a/fs/xfs/xfs_reflink.c b/fs/xfs/xfs_reflink.c
> index 4225b5e67b17..4702dd800ab8 100644
> --- a/fs/xfs/xfs_reflink.c
> +++ b/fs/xfs/xfs_reflink.c
> @@ -264,9 +264,9 @@ int
> xfs_reflink_reserve_cow(
> struct xfs_inode *ip,
> struct xfs_bmbt_irec *imap,
> - bool *shared)
> + bool *shared,
> + bool always_cow)
> {
> - struct xfs_ifork *ifp = XFS_IFORK_PTR(ip, XFS_COW_FORK);
> struct xfs_bmbt_irec got;
> int error = 0;
> bool eof = false, trimmed;
> @@ -280,26 +280,30 @@ xfs_reflink_reserve_cow(
> * extent list is generally faster than going out to the shared extent
> * tree.
> */
> -
> - if (!xfs_iext_lookup_extent(ip, ifp, imap->br_startoff, &idx, &got))
> + if (!ip->i_cowfp) {
> + ASSERT(always_cow);
> + xfs_ifork_init_cow(ip);
> eof = true;
> - if (!eof && got.br_startoff <= imap->br_startoff) {
> - trace_xfs_reflink_cow_found(ip, imap);
> - xfs_trim_extent(imap, got.br_startoff, got.br_blockcount);
> + } else {
> + if (!xfs_iext_lookup_extent(ip, ip->i_cowfp, imap->br_startoff,
> + &idx, &got))
> + eof = true;
> + if (!eof && got.br_startoff <= imap->br_startoff) {
> + trace_xfs_reflink_cow_found(ip, imap);
> + xfs_trim_extent(imap, got.br_startoff,
> + got.br_blockcount);
> +
> + *shared = true;
> + return 0;
> + }
>
> - *shared = true;
> - return 0;
> + /* Trim the mapping to the nearest shared extent boundary. */
> + error = xfs_reflink_trim_around_shared(ip, imap, shared,
> + &trimmed);
> + if (error || !*shared)
> + return error;
> }
>
> - /* Trim the mapping to the nearest shared extent boundary. */
> - error = xfs_reflink_trim_around_shared(ip, imap, shared, &trimmed);
> - if (error)
> - return error;
> -
> - /* Not shared? Just report the (potentially capped) extent. */
> - if (!*shared)
> - return 0;
> -
> /*
> * Fork all the shared blocks from our write offset until the end of
> * the extent.
> @@ -383,7 +387,8 @@ xfs_reflink_allocate_cow(
> struct xfs_inode *ip,
> struct xfs_bmbt_irec *imap,
> bool *shared,
> - uint *lockmode)
> + uint *lockmode,
> + bool always_cow)
> {
> struct xfs_mount *mp = ip->i_mount;
> xfs_fileoff_t offset_fsb = imap->br_startoff;
> @@ -399,15 +404,19 @@ xfs_reflink_allocate_cow(
> xfs_extnum_t idx;
>
> retry:
> - ASSERT(xfs_is_reflink_inode(ip));
> + ASSERT(always_cow | xfs_is_reflink_inode(ip));
> ASSERT(xfs_isilocked(ip, XFS_ILOCK_EXCL | XFS_ILOCK_SHARED));
>
> + if (!ip->i_cowfp) {
> + ASSERT(always_cow);
> + xfs_ifork_init_cow(ip);
> +
> /*
> * Even if the extent is not shared we might have a preallocation for
> * it in the COW fork. If so use it.
> */
> - if (xfs_iext_lookup_extent(ip, ip->i_cowfp, offset_fsb, &idx, &got) &&
> - got.br_startoff <= offset_fsb) {
> + } else if (xfs_iext_lookup_extent(ip, ip->i_cowfp, offset_fsb, &idx,
> + &got) && got.br_startoff <= offset_fsb) {
> *shared = true;
>
> /* If we have a real allocation in the COW fork we're done. */
> @@ -418,7 +427,7 @@ xfs_reflink_allocate_cow(
> }
>
> xfs_trim_extent(imap, got.br_startoff, got.br_blockcount);
> - } else {
> + } else if (!always_cow) {
> error = xfs_reflink_trim_around_shared(ip, imap, shared, &trimmed);
> if (error || !*shared)
> goto out;
> @@ -684,6 +693,7 @@ xfs_reflink_end_cow(
> xfs_fileoff_t offset_fsb;
> xfs_fileoff_t end_fsb;
> xfs_fsblock_t firstfsb;
> + xfs_off_t new_size;
> struct xfs_defer_ops dfops;
> int error;
> unsigned int resblks;
> @@ -693,7 +703,7 @@ xfs_reflink_end_cow(
> trace_xfs_reflink_end_cow(ip, offset, count);
>
> /* No COW extents? That's easy! */
> - if (ifp->if_bytes == 0)
> + if (!ifp || ifp->if_bytes == 0)
> return 0;
>
> offset_fsb = XFS_B_TO_FSBT(ip->i_mount, offset);
> @@ -776,6 +786,17 @@ xfs_reflink_end_cow(
> break;
> }
>
> + /*
> + * Update the on-disk inode size if we completed an operation outside
> + * of the inode size. This can only happen for atomic writes, and not
> + * for actual reflinked files.
> + */
> + new_size = xfs_new_eof(ip, offset + count);
> + if (new_size) {
> + ip->i_d.di_size = new_size;
> + xfs_trans_log_inode(tp, ip, XFS_ILOG_CORE);
> + }
> +
> error = xfs_trans_commit(tp);
> xfs_iunlock(ip, XFS_ILOCK_EXCL);
> if (error)
> diff --git a/fs/xfs/xfs_reflink.h b/fs/xfs/xfs_reflink.h
> index 9416279b3c89..0360e2c0f3a5 100644
> --- a/fs/xfs/xfs_reflink.h
> +++ b/fs/xfs/xfs_reflink.h
> @@ -27,9 +27,10 @@ extern int xfs_reflink_trim_around_shared(struct xfs_inode *ip,
> struct xfs_bmbt_irec *irec, bool *shared, bool *trimmed);
>
> extern int xfs_reflink_reserve_cow(struct xfs_inode *ip,
> - struct xfs_bmbt_irec *imap, bool *shared);
> + struct xfs_bmbt_irec *imap, bool *shared, bool always_cow);
> extern int xfs_reflink_allocate_cow(struct xfs_inode *ip,
> - struct xfs_bmbt_irec *imap, bool *shared, uint *lockmode);
> + struct xfs_bmbt_irec *imap, bool *shared, uint *lockmode,
> + bool always_cow);
manpages/xfstests needed, but the rest of this looks more or less sane.
--D
> extern int xfs_reflink_convert_cow(struct xfs_inode *ip, xfs_off_t offset,
> xfs_off_t count);
> extern bool xfs_reflink_find_cow_mapping(struct xfs_inode *ip, xfs_off_t offset,
> --
> 2.11.0
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-xfs" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply
* Re: [PATCH] blkcg: allocate struct blkcg_gq outside request queue spinlock
From: Tahsin Erdogan @ 2017-02-28 23:51 UTC (permalink / raw)
To: Tejun Heo; +Cc: Jens Axboe, linux-block, David Rientjes, linux-kernel
In-Reply-To: <20170228224728.GJ15287@htj.duckdns.org>
On Tue, Feb 28, 2017 at 2:47 PM, Tejun Heo <tj@kernel.org> wrote:
>> + if (!blkcg_policy_enabled(q, pol)) {
>> + ret = -EOPNOTSUPP;
>> + goto fail;
>
> Pulling this out of the queue_lock doesn't seem safe to me. This
> function may end up calling into callbacks of disabled policies this
> way.
I will move this to within the lock. To make things safe, I am also
thinking of rechecking both blkcg_policy_enabled() and
blk_queue_bypass() after reacquiring the locks in each iteration.
>> + parent = blkcg_parent(blkcg);
>> + while (parent && !__blkg_lookup(parent, q, false)) {
>> + pos = parent;
>> + parent = blkcg_parent(parent);
>> + }
>
> Hmm... how about adding @new_blkg to blkg_lookup_create() and calling
> it with non-NULL @new_blkg until it succeeds? Wouldn't that be
> simpler?
>
>> +
>> + new_blkg = blkg_alloc(pos, q, GFP_KERNEL);
The challenge with that approach is creating a new_blkg with the right
blkcg before passing to blkg_lookup_create(). blkg_lookup_create()
walks down the hierarchy and will try to fill the first missing entry
and the preallocated new_blkg must have been created with the right
blkcg (feel free to send a code fragment if you think I am
misunderstanding the suggestion).
^ permalink raw reply
* Re: [PATCH v2 11/13] md: raid10: don't use bio's vec table to manage resync pages
From: Shaohua Li @ 2017-02-28 23:43 UTC (permalink / raw)
To: Ming Lei; +Cc: Jens Axboe, linux-raid, linux-block, Christoph Hellwig
In-Reply-To: <1488296503-4987-12-git-send-email-tom.leiming@gmail.com>
On Tue, Feb 28, 2017 at 11:41:41PM +0800, Ming Lei wrote:
> Now we allocate one page array for managing resync pages, instead
> of using bio's vec table to do that, and the old way is very hacky
> and won't work any more if multipage bvec is enabled.
>
> The introduced cost is that we need to allocate (128 + 16) * copies
> bytes per r10_bio, and it is fine because the inflight r10_bio for
> resync shouldn't be much, as pointed by Shaohua.
>
> Also bio_reset() in raid10_sync_request() and reshape_request()
> are removed because all bios are freshly new now in these functions
> and not necessary to reset any more.
>
> This patch can be thought as cleanup too.
>
> Suggested-by: Shaohua Li <shli@kernel.org>
> Signed-off-by: Ming Lei <tom.leiming@gmail.com>
> ---
> drivers/md/raid10.c | 125 ++++++++++++++++++++++++++++++----------------------
> 1 file changed, 73 insertions(+), 52 deletions(-)
>
> diff --git a/drivers/md/raid10.c b/drivers/md/raid10.c
> index a9ddd4f14008..f887b21332e7 100644
> --- a/drivers/md/raid10.c
> +++ b/drivers/md/raid10.c
> @@ -110,6 +110,16 @@ static void end_reshape(struct r10conf *conf);
> #define raid10_log(md, fmt, args...) \
> do { if ((md)->queue) blk_add_trace_msg((md)->queue, "raid10 " fmt, ##args); } while (0)
>
> +static inline struct resync_pages *get_resync_pages(struct bio *bio)
> +{
> + return bio->bi_private;
> +}
> +
> +static inline struct r10bio *get_resync_r10bio(struct bio *bio)
> +{
> + return get_resync_pages(bio)->raid_bio;
> +}
Same applies to raid10 too. embedded the resync_pages into r10bio, possibly a
pointer. Merge the 11, 12, 13 into a single patch.
Thanks,
Shaohua
> +
> static void * r10bio_pool_alloc(gfp_t gfp_flags, void *data)
> {
> struct r10conf *conf = data;
> @@ -140,11 +150,11 @@ static void r10bio_pool_free(void *r10_bio, void *data)
> static void * r10buf_pool_alloc(gfp_t gfp_flags, void *data)
> {
> struct r10conf *conf = data;
> - struct page *page;
> struct r10bio *r10_bio;
> struct bio *bio;
> - int i, j;
> - int nalloc;
> + int j;
> + int nalloc, nalloc_rp;
> + struct resync_pages *rps;
>
> r10_bio = r10bio_pool_alloc(gfp_flags, conf);
> if (!r10_bio)
> @@ -156,6 +166,15 @@ static void * r10buf_pool_alloc(gfp_t gfp_flags, void *data)
> else
> nalloc = 2; /* recovery */
>
> + /* allocate once for all bios */
> + if (!conf->have_replacement)
> + nalloc_rp = nalloc;
> + else
> + nalloc_rp = nalloc * 2;
> + rps = kmalloc(sizeof(struct resync_pages) * nalloc_rp, gfp_flags);
> + if (!rps)
> + goto out_free_r10bio;
> +
> /*
> * Allocate bios.
> */
> @@ -175,36 +194,40 @@ static void * r10buf_pool_alloc(gfp_t gfp_flags, void *data)
> * Allocate RESYNC_PAGES data pages and attach them
> * where needed.
> */
> - for (j = 0 ; j < nalloc; j++) {
> + for (j = 0; j < nalloc; j++) {
> struct bio *rbio = r10_bio->devs[j].repl_bio;
> + struct resync_pages *rp, *rp_repl;
> +
> + rp = &rps[j];
> + if (rbio)
> + rp_repl = &rps[nalloc + j];
> +
> bio = r10_bio->devs[j].bio;
> - for (i = 0; i < RESYNC_PAGES; i++) {
> - if (j > 0 && !test_bit(MD_RECOVERY_SYNC,
> - &conf->mddev->recovery)) {
> - /* we can share bv_page's during recovery
> - * and reshape */
> - struct bio *rbio = r10_bio->devs[0].bio;
> - page = rbio->bi_io_vec[i].bv_page;
> - get_page(page);
> - } else
> - page = alloc_page(gfp_flags);
> - if (unlikely(!page))
> +
> + if (!j || test_bit(MD_RECOVERY_SYNC,
> + &conf->mddev->recovery)) {
> + if (resync_alloc_pages(rp, gfp_flags))
> goto out_free_pages;
> + } else {
> + memcpy(rp, &rps[0], sizeof(*rp));
> + resync_get_all_pages(rp);
> + }
>
> - bio->bi_io_vec[i].bv_page = page;
> - if (rbio)
> - rbio->bi_io_vec[i].bv_page = page;
> + rp->idx = 0;
> + rp->raid_bio = r10_bio;
> + bio->bi_private = rp;
> + if (rbio) {
> + memcpy(rp_repl, rp, sizeof(*rp));
> + rbio->bi_private = rp_repl;
> }
> }
>
> return r10_bio;
>
> out_free_pages:
> - for ( ; i > 0 ; i--)
> - safe_put_page(bio->bi_io_vec[i-1].bv_page);
> - while (j--)
> - for (i = 0; i < RESYNC_PAGES ; i++)
> - safe_put_page(r10_bio->devs[j].bio->bi_io_vec[i].bv_page);
> + while (--j >= 0)
> + resync_free_pages(&rps[j * 2]);
> +
> j = 0;
> out_free_bio:
> for ( ; j < nalloc; j++) {
> @@ -213,30 +236,34 @@ static void * r10buf_pool_alloc(gfp_t gfp_flags, void *data)
> if (r10_bio->devs[j].repl_bio)
> bio_put(r10_bio->devs[j].repl_bio);
> }
> + kfree(rps);
> +out_free_r10bio:
> r10bio_pool_free(r10_bio, conf);
> return NULL;
> }
>
> static void r10buf_pool_free(void *__r10_bio, void *data)
> {
> - int i;
> struct r10conf *conf = data;
> struct r10bio *r10bio = __r10_bio;
> int j;
> + struct resync_pages *rp = NULL;
>
> - for (j=0; j < conf->copies; j++) {
> + for (j = conf->copies; j--; ) {
> struct bio *bio = r10bio->devs[j].bio;
> - if (bio) {
> - for (i = 0; i < RESYNC_PAGES; i++) {
> - safe_put_page(bio->bi_io_vec[i].bv_page);
> - bio->bi_io_vec[i].bv_page = NULL;
> - }
> - bio_put(bio);
> - }
> +
> + rp = get_resync_pages(bio);
> + resync_free_pages(rp);
> + bio_put(bio);
> +
> bio = r10bio->devs[j].repl_bio;
> if (bio)
> bio_put(bio);
> }
> +
> + /* resync pages array stored in the 1st bio's .bi_private */
> + kfree(rp);
> +
> r10bio_pool_free(r10bio, conf);
> }
>
> @@ -1935,7 +1962,7 @@ static void __end_sync_read(struct r10bio *r10_bio, struct bio *bio, int d)
>
> static void end_sync_read(struct bio *bio)
> {
> - struct r10bio *r10_bio = bio->bi_private;
> + struct r10bio *r10_bio = get_resync_r10bio(bio);
> struct r10conf *conf = r10_bio->mddev->private;
> int d = find_bio_disk(conf, r10_bio, bio, NULL, NULL);
>
> @@ -1944,6 +1971,7 @@ static void end_sync_read(struct bio *bio)
>
> static void end_reshape_read(struct bio *bio)
> {
> + /* reshape read bio isn't allocated from r10buf_pool */
> struct r10bio *r10_bio = bio->bi_private;
>
> __end_sync_read(r10_bio, bio, r10_bio->read_slot);
> @@ -1978,7 +2006,7 @@ static void end_sync_request(struct r10bio *r10_bio)
>
> static void end_sync_write(struct bio *bio)
> {
> - struct r10bio *r10_bio = bio->bi_private;
> + struct r10bio *r10_bio = get_resync_r10bio(bio);
> struct mddev *mddev = r10_bio->mddev;
> struct r10conf *conf = mddev->private;
> int d;
> @@ -2058,6 +2086,7 @@ static void sync_request_write(struct mddev *mddev, struct r10bio *r10_bio)
> for (i=0 ; i < conf->copies ; i++) {
> int j, d;
> struct md_rdev *rdev;
> + struct resync_pages *rp;
>
> tbio = r10_bio->devs[i].bio;
>
> @@ -2099,11 +2128,13 @@ static void sync_request_write(struct mddev *mddev, struct r10bio *r10_bio)
> * First we need to fixup bv_offset, bv_len and
> * bi_vecs, as the read request might have corrupted these
> */
> + rp = get_resync_pages(tbio);
> bio_reset(tbio);
>
> tbio->bi_vcnt = vcnt;
> tbio->bi_iter.bi_size = fbio->bi_iter.bi_size;
> - tbio->bi_private = r10_bio;
> + rp->raid_bio = r10_bio;
> + tbio->bi_private = rp;
> tbio->bi_iter.bi_sector = r10_bio->devs[i].addr;
> tbio->bi_end_io = end_sync_write;
> bio_set_op_attrs(tbio, REQ_OP_WRITE, 0);
> @@ -3171,10 +3202,8 @@ static sector_t raid10_sync_request(struct mddev *mddev, sector_t sector_nr,
> }
> }
> bio = r10_bio->devs[0].bio;
> - bio_reset(bio);
> bio->bi_next = biolist;
> biolist = bio;
> - bio->bi_private = r10_bio;
> bio->bi_end_io = end_sync_read;
> bio_set_op_attrs(bio, REQ_OP_READ, 0);
> if (test_bit(FailFast, &rdev->flags))
> @@ -3198,10 +3227,8 @@ static sector_t raid10_sync_request(struct mddev *mddev, sector_t sector_nr,
>
> if (!test_bit(In_sync, &mrdev->flags)) {
> bio = r10_bio->devs[1].bio;
> - bio_reset(bio);
> bio->bi_next = biolist;
> biolist = bio;
> - bio->bi_private = r10_bio;
> bio->bi_end_io = end_sync_write;
> bio_set_op_attrs(bio, REQ_OP_WRITE, 0);
> bio->bi_iter.bi_sector = to_addr
> @@ -3226,10 +3253,8 @@ static sector_t raid10_sync_request(struct mddev *mddev, sector_t sector_nr,
> if (mreplace == NULL || bio == NULL ||
> test_bit(Faulty, &mreplace->flags))
> break;
> - bio_reset(bio);
> bio->bi_next = biolist;
> biolist = bio;
> - bio->bi_private = r10_bio;
> bio->bi_end_io = end_sync_write;
> bio_set_op_attrs(bio, REQ_OP_WRITE, 0);
> bio->bi_iter.bi_sector = to_addr +
> @@ -3351,7 +3376,6 @@ static sector_t raid10_sync_request(struct mddev *mddev, sector_t sector_nr,
> r10_bio->devs[i].repl_bio->bi_end_io = NULL;
>
> bio = r10_bio->devs[i].bio;
> - bio_reset(bio);
> bio->bi_error = -EIO;
> rcu_read_lock();
> rdev = rcu_dereference(conf->mirrors[d].rdev);
> @@ -3376,7 +3400,6 @@ static sector_t raid10_sync_request(struct mddev *mddev, sector_t sector_nr,
> atomic_inc(&r10_bio->remaining);
> bio->bi_next = biolist;
> biolist = bio;
> - bio->bi_private = r10_bio;
> bio->bi_end_io = end_sync_read;
> bio_set_op_attrs(bio, REQ_OP_READ, 0);
> if (test_bit(FailFast, &conf->mirrors[d].rdev->flags))
> @@ -3395,13 +3418,11 @@ static sector_t raid10_sync_request(struct mddev *mddev, sector_t sector_nr,
>
> /* Need to set up for writing to the replacement */
> bio = r10_bio->devs[i].repl_bio;
> - bio_reset(bio);
> bio->bi_error = -EIO;
>
> sector = r10_bio->devs[i].addr;
> bio->bi_next = biolist;
> biolist = bio;
> - bio->bi_private = r10_bio;
> bio->bi_end_io = end_sync_write;
> bio_set_op_attrs(bio, REQ_OP_WRITE, 0);
> if (test_bit(FailFast, &conf->mirrors[d].rdev->flags))
> @@ -3440,7 +3461,7 @@ static sector_t raid10_sync_request(struct mddev *mddev, sector_t sector_nr,
> if (len == 0)
> break;
> for (bio= biolist ; bio ; bio=bio->bi_next) {
> - page = bio->bi_io_vec[bio->bi_vcnt].bv_page;
> + page = resync_fetch_page(get_resync_pages(bio));
> /*
> * won't fail because the vec table is big enough
> * to hold all these pages
> @@ -3449,7 +3470,7 @@ static sector_t raid10_sync_request(struct mddev *mddev, sector_t sector_nr,
> }
> nr_sectors += len>>9;
> sector_nr += len>>9;
> - } while (biolist->bi_vcnt < RESYNC_PAGES);
> + } while (resync_page_available(get_resync_pages(biolist)));
> r10_bio->sectors = nr_sectors;
>
> while (biolist) {
> @@ -3457,7 +3478,7 @@ static sector_t raid10_sync_request(struct mddev *mddev, sector_t sector_nr,
> biolist = biolist->bi_next;
>
> bio->bi_next = NULL;
> - r10_bio = bio->bi_private;
> + r10_bio = get_resync_r10bio(bio);
> r10_bio->sectors = nr_sectors;
>
> if (bio->bi_end_io == end_sync_read) {
> @@ -4352,6 +4373,7 @@ static sector_t reshape_request(struct mddev *mddev, sector_t sector_nr,
> struct bio *blist;
> struct bio *bio, *read_bio;
> int sectors_done = 0;
> + struct page **pages;
>
> if (sector_nr == 0) {
> /* If restarting in the middle, skip the initial sectors */
> @@ -4502,11 +4524,9 @@ static sector_t reshape_request(struct mddev *mddev, sector_t sector_nr,
> if (!rdev2 || test_bit(Faulty, &rdev2->flags))
> continue;
>
> - bio_reset(b);
> b->bi_bdev = rdev2->bdev;
> b->bi_iter.bi_sector = r10_bio->devs[s/2].addr +
> rdev2->new_data_offset;
> - b->bi_private = r10_bio;
> b->bi_end_io = end_reshape_write;
> bio_set_op_attrs(b, REQ_OP_WRITE, 0);
> b->bi_next = blist;
> @@ -4516,8 +4536,9 @@ static sector_t reshape_request(struct mddev *mddev, sector_t sector_nr,
> /* Now add as many pages as possible to all of these bios. */
>
> nr_sectors = 0;
> + pages = get_resync_pages(r10_bio->devs[0].bio)->pages;
> for (s = 0 ; s < max_sectors; s += PAGE_SIZE >> 9) {
> - struct page *page = r10_bio->devs[0].bio->bi_io_vec[s/(PAGE_SIZE>>9)].bv_page;
> + struct page *page = pages[s / (PAGE_SIZE >> 9)];
> int len = (max_sectors - s) << 9;
> if (len > PAGE_SIZE)
> len = PAGE_SIZE;
> @@ -4701,7 +4722,7 @@ static int handle_reshape_read_error(struct mddev *mddev,
>
> static void end_reshape_write(struct bio *bio)
> {
> - struct r10bio *r10_bio = bio->bi_private;
> + struct r10bio *r10_bio = get_resync_r10bio(bio);
> struct mddev *mddev = r10_bio->mddev;
> struct r10conf *conf = mddev->private;
> int d;
> --
> 2.7.4
>
^ permalink raw reply
* Re: [PATCH v2 13/13] md: raid10: avoid direct access to bvec table in handle_reshape_read_error
From: Shaohua Li @ 2017-02-28 23:46 UTC (permalink / raw)
To: Ming Lei; +Cc: Jens Axboe, linux-raid, linux-block, Christoph Hellwig
In-Reply-To: <1488296503-4987-14-git-send-email-tom.leiming@gmail.com>
On Tue, Feb 28, 2017 at 11:41:43PM +0800, Ming Lei wrote:
> The cost is 128bytes(8*16) stack space in kernel thread context, and
> just use the bio helper to retrieve pages from bio.
>
> Signed-off-by: Ming Lei <tom.leiming@gmail.com>
> ---
> drivers/md/raid10.c | 12 ++++++++++--
> 1 file changed, 10 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/md/raid10.c b/drivers/md/raid10.c
> index 0b97631e3905..6ffb64ab45f8 100644
> --- a/drivers/md/raid10.c
> +++ b/drivers/md/raid10.c
> @@ -4670,7 +4670,15 @@ static int handle_reshape_read_error(struct mddev *mddev,
> struct r10bio *r10b = &on_stack.r10_bio;
> int slot = 0;
> int idx = 0;
> - struct bio_vec *bvec = r10_bio->master_bio->bi_io_vec;
> + struct bio_vec *bvl;
> + struct page *pages[RESYNC_PAGES];
> +
> + /*
> + * This bio is allocated in reshape_request(), and size
> + * is still RESYNC_PAGES
> + */
> + bio_for_each_segment_all(bvl, r10_bio->master_bio, idx)
> + pages[idx] = bvl->bv_page;
The reshape bio is doing IO against the memory we allocated for r10_bio, I'm
wondering why we can't get the pages from r10_bio. In this way, we don't need
access the bio_vec any more.
Thanks,
Shaohua
> r10b->sector = r10_bio->sector;
> __raid10_find_phys(&conf->prev, r10b);
> @@ -4699,7 +4707,7 @@ static int handle_reshape_read_error(struct mddev *mddev,
> success = sync_page_io(rdev,
> addr,
> s << 9,
> - bvec[idx].bv_page,
> + pages[idx],
> REQ_OP_READ, 0, false);
> rdev_dec_pending(rdev, mddev);
> rcu_read_lock();
> --
> 2.7.4
>
^ permalink raw reply
* Re: [PATCH v2 06/13] md: raid1: don't use bio's vec table to manage resync pages
From: Shaohua Li @ 2017-02-28 23:37 UTC (permalink / raw)
To: Ming Lei; +Cc: Jens Axboe, linux-raid, linux-block, Christoph Hellwig
In-Reply-To: <1488296503-4987-7-git-send-email-tom.leiming@gmail.com>
On Tue, Feb 28, 2017 at 11:41:36PM +0800, Ming Lei wrote:
> Now we allocate one page array for managing resync pages, instead
> of using bio's vec table to do that, and the old way is very hacky
> and won't work any more if multipage bvec is enabled.
>
> The introduced cost is that we need to allocate (128 + 16) * raid_disks
> bytes per r1_bio, and it is fine because the inflight r1_bio for
> resync shouldn't be much, as pointed by Shaohua.
>
> Also the bio_reset() in raid1_sync_request() is removed because
> all bios are freshly new now and not necessary to reset any more.
>
> This patch can be thought as a cleanup too
>
> Suggested-by: Shaohua Li <shli@kernel.org>
> Signed-off-by: Ming Lei <tom.leiming@gmail.com>
> ---
> drivers/md/raid1.c | 83 ++++++++++++++++++++++++++++++++++--------------------
> 1 file changed, 53 insertions(+), 30 deletions(-)
>
> diff --git a/drivers/md/raid1.c b/drivers/md/raid1.c
> index c442b4657e2f..900144f39630 100644
> --- a/drivers/md/raid1.c
> +++ b/drivers/md/raid1.c
> @@ -77,6 +77,16 @@ static void lower_barrier(struct r1conf *conf, sector_t sector_nr);
> #define raid1_log(md, fmt, args...) \
> do { if ((md)->queue) blk_add_trace_msg((md)->queue, "raid1 " fmt, ##args); } while (0)
>
> +static inline struct resync_pages *get_resync_pages(struct bio *bio)
> +{
> + return bio->bi_private;
> +}
> +
> +static inline struct r1bio *get_resync_r1bio(struct bio *bio)
> +{
> + return get_resync_pages(bio)->raid_bio;
> +}
This is a weird between bio, r1bio and the resync_pages. I'd like the pages are
embedded in r1bio. Maybe a pointer of r1bio to the pages. It's cleaner and more
straightforward.
I think the patch 6, 7 8 should be in a same patch. Otherwise bisect will be broken.
Thanks,
Shaohua
^ permalink raw reply
* Re: [PATCH v2 09/13] md: raid1: use bio_segments_all()
From: Shaohua Li @ 2017-02-28 23:42 UTC (permalink / raw)
To: Ming Lei; +Cc: Jens Axboe, linux-raid, linux-block, Christoph Hellwig
In-Reply-To: <1488296503-4987-10-git-send-email-tom.leiming@gmail.com>
On Tue, Feb 28, 2017 at 11:41:39PM +0800, Ming Lei wrote:
> Use this helper, instead of direct access to .bi_vcnt.
what We really need to do for the behind IO is:
- allocate memory and copy bio data to the memory
- let behind bio do IO against the memory
The behind bio doesn't need to have the exactly same bio_vec setting. If we
just track the new memory, we don't need use the bio_segments_all and access
bio_vec too.
Thanks,
Shaohua
> Signed-off-by: Ming Lei <tom.leiming@gmail.com>
> ---
> drivers/md/raid1.c | 7 ++++---
> 1 file changed, 4 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/md/raid1.c b/drivers/md/raid1.c
> index 316bd6dd6cc1..7396c99ff7b1 100644
> --- a/drivers/md/raid1.c
> +++ b/drivers/md/raid1.c
> @@ -1091,7 +1091,8 @@ static void alloc_behind_pages(struct bio *bio, struct r1bio *r1_bio)
> {
> int i;
> struct bio_vec *bvec;
> - struct bio_vec *bvecs = kzalloc(bio->bi_vcnt * sizeof(struct bio_vec),
> + unsigned vcnt = bio_segments_all(bio);
> + struct bio_vec *bvecs = kzalloc(vcnt * sizeof(struct bio_vec),
> GFP_NOIO);
> if (unlikely(!bvecs))
> return;
> @@ -1107,12 +1108,12 @@ static void alloc_behind_pages(struct bio *bio, struct r1bio *r1_bio)
> kunmap(bvec->bv_page);
> }
> r1_bio->behind_bvecs = bvecs;
> - r1_bio->behind_page_count = bio->bi_vcnt;
> + r1_bio->behind_page_count = vcnt;
> set_bit(R1BIO_BehindIO, &r1_bio->state);
> return;
>
> do_sync_io:
> - for (i = 0; i < bio->bi_vcnt; i++)
> + for (i = 0; i < vcnt; i++)
> if (bvecs[i].bv_page)
> put_page(bvecs[i].bv_page);
> kfree(bvecs);
> --
> 2.7.4
>
^ permalink raw reply
* Re: [PATCH v2 08/13] md: raid1: use bio helper in process_checks()
From: Shaohua Li @ 2017-02-28 23:39 UTC (permalink / raw)
To: Ming Lei; +Cc: Jens Axboe, linux-raid, linux-block, Christoph Hellwig
In-Reply-To: <1488296503-4987-9-git-send-email-tom.leiming@gmail.com>
On Tue, Feb 28, 2017 at 11:41:38PM +0800, Ming Lei wrote:
> Avoid to direct access to bvec table.
>
> Signed-off-by: Ming Lei <tom.leiming@gmail.com>
> ---
> drivers/md/raid1.c | 12 ++++++++----
> 1 file changed, 8 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/md/raid1.c b/drivers/md/raid1.c
> index d0cb5c026506..316bd6dd6cc1 100644
> --- a/drivers/md/raid1.c
> +++ b/drivers/md/raid1.c
> @@ -2108,6 +2108,7 @@ static void process_checks(struct r1bio *r1_bio)
> int j;
> int size;
> int error;
> + struct bio_vec *bi;
> struct bio *b = r1_bio->bios[i];
> struct resync_pages *rp = get_resync_pages(b);
> if (b->bi_end_io != end_sync_read)
> @@ -2126,9 +2127,7 @@ static void process_checks(struct r1bio *r1_bio)
> b->bi_private = rp;
>
> size = b->bi_iter.bi_size;
> - for (j = 0; j < vcnt ; j++) {
> - struct bio_vec *bi;
> - bi = &b->bi_io_vec[j];
> + bio_for_each_segment_all(bi, b, j) {
> bi->bv_offset = 0;
> if (size > PAGE_SIZE)
> bi->bv_len = PAGE_SIZE;
> @@ -2152,17 +2151,22 @@ static void process_checks(struct r1bio *r1_bio)
> int error = sbio->bi_error;
> struct page **ppages = get_resync_pages(pbio)->pages;
> struct page **spages = get_resync_pages(sbio)->pages;
> + struct bio_vec *bi;
> + int page_len[RESYNC_PAGES];
>
> if (sbio->bi_end_io != end_sync_read)
> continue;
> /* Now we can 'fixup' the error value */
> sbio->bi_error = 0;
>
> + bio_for_each_segment_all(bi, sbio, j)
> + page_len[j] = bi->bv_len;
> +
If we record length for each page in resync_pages (don't need offset, because
it should be 0), we don't need to access the bio_vec too.
Thanks,
Shaohua
^ permalink raw reply
* [PATCH 8/8] nowait aio: btrfs
From: Goldwyn Rodrigues @ 2017-02-28 23:36 UTC (permalink / raw)
To: jack
Cc: hch, linux-fsdevel, linux-block, linux-btrfs, linux-ext4,
linux-xfs, Goldwyn Rodrigues
In-Reply-To: <20170228233610.25456-1-rgoldwyn@suse.de>
From: Goldwyn Rodrigues <rgoldwyn@suse.com>
Return EAGAIN if any of the following checks fail
+ i_rwsem is not lockable
+ NODATACOW or PREALLOC is not set
+ Cannot nocow at the desired location
+ Writing beyond end of file which is not allocated
Signed-off-by: Goldwyn Rodrigues <rgoldwyn@suse.com>
---
fs/btrfs/file.c | 25 ++++++++++++++++++++-----
fs/btrfs/inode.c | 3 +++
2 files changed, 23 insertions(+), 5 deletions(-)
diff --git a/fs/btrfs/file.c b/fs/btrfs/file.c
index b5c5da2..8640280 100644
--- a/fs/btrfs/file.c
+++ b/fs/btrfs/file.c
@@ -1819,12 +1819,29 @@ static ssize_t btrfs_file_write_iter(struct kiocb *iocb,
ssize_t num_written = 0;
bool sync = (file->f_flags & O_DSYNC) || IS_SYNC(file->f_mapping->host);
ssize_t err;
- loff_t pos;
- size_t count;
+ loff_t pos = iocb->ki_pos;
+ size_t count = iov_iter_count(from);
loff_t oldsize;
int clean_page = 0;
- inode_lock(inode);
+ if ((iocb->ki_flags & IOCB_NOWAIT) &&
+ (iocb->ki_flags & IOCB_DIRECT)) {
+ /* Don't sleep on inode rwsem */
+ if (!inode_trylock(inode))
+ return -EAGAIN;
+ /*
+ * We will allocate space in case nodatacow is not set,
+ * so bail
+ */
+ if (!(BTRFS_I(inode)->flags & (BTRFS_INODE_NODATACOW |
+ BTRFS_INODE_PREALLOC)) ||
+ check_can_nocow(inode, pos, &count) <= 0) {
+ inode_unlock(inode);
+ return -EAGAIN;
+ }
+ } else
+ inode_lock(inode);
+
err = generic_write_checks(iocb, from);
if (err <= 0) {
inode_unlock(inode);
@@ -1858,8 +1875,6 @@ static ssize_t btrfs_file_write_iter(struct kiocb *iocb,
*/
update_time_for_write(inode);
- pos = iocb->ki_pos;
- count = iov_iter_count(from);
start_pos = round_down(pos, fs_info->sectorsize);
oldsize = i_size_read(inode);
if (start_pos > oldsize) {
diff --git a/fs/btrfs/inode.c b/fs/btrfs/inode.c
index 1e861a0..c5041ea 100644
--- a/fs/btrfs/inode.c
+++ b/fs/btrfs/inode.c
@@ -8681,6 +8681,9 @@ static ssize_t btrfs_direct_IO(struct kiocb *iocb, struct iov_iter *iter)
if (offset + count <= inode->i_size) {
inode_unlock(inode);
relock = true;
+ } else if (iocb->ki_flags & IOCB_NOWAIT) {
+ ret = -EAGAIN;
+ goto out;
}
ret = btrfs_delalloc_reserve_space(inode, offset, count);
if (ret)
--
2.10.2
^ permalink raw reply related
* [PATCH 7/8] nowait aio: xfs
From: Goldwyn Rodrigues @ 2017-02-28 23:36 UTC (permalink / raw)
To: jack
Cc: hch, linux-fsdevel, linux-block, linux-btrfs, linux-ext4,
linux-xfs, Goldwyn Rodrigues
In-Reply-To: <20170228233610.25456-1-rgoldwyn@suse.de>
From: Goldwyn Rodrigues <rgoldwyn@suse.com>
If IOCB_NOWAIT is set, bail if the i_rwsem is not lockable
immediately.
IF IOMAP_NOWAIT is set, return EAGAIN in xfs_file_iomap_begin
if it needs allocation either due to file extending, writing to a hole,
or COW.
Signed-off-by: Goldwyn Rodrigues <rgoldwyn@suse.com>
---
fs/xfs/xfs_file.c | 9 +++++++--
fs/xfs/xfs_iomap.c | 9 +++++++++
2 files changed, 16 insertions(+), 2 deletions(-)
diff --git a/fs/xfs/xfs_file.c b/fs/xfs/xfs_file.c
index bbb9eb6..7e16a83 100644
--- a/fs/xfs/xfs_file.c
+++ b/fs/xfs/xfs_file.c
@@ -528,12 +528,17 @@ xfs_file_dio_aio_write(
((iocb->ki_pos + count) & mp->m_blockmask)) {
unaligned_io = 1;
iolock = XFS_IOLOCK_EXCL;
+ if (iocb->ki_flags & IOCB_NOWAIT)
+ return -EAGAIN;
} else {
iolock = XFS_IOLOCK_SHARED;
}
- xfs_ilock(ip, iolock);
-
+ if (!xfs_ilock_nowait(ip, iolock)) {
+ if (iocb->ki_flags & IOCB_NOWAIT)
+ return -EAGAIN;
+ xfs_ilock(ip, iolock);
+ }
ret = xfs_file_aio_write_checks(iocb, from, &iolock);
if (ret)
goto out;
diff --git a/fs/xfs/xfs_iomap.c b/fs/xfs/xfs_iomap.c
index 1aa3abd..84f981a 100644
--- a/fs/xfs/xfs_iomap.c
+++ b/fs/xfs/xfs_iomap.c
@@ -1020,6 +1020,11 @@ xfs_file_iomap_begin(
if ((flags & IOMAP_REPORT) ||
(xfs_is_reflink_inode(ip) &&
(flags & IOMAP_WRITE) && (flags & IOMAP_DIRECT))) {
+ /* Allocations due to reflinks */
+ if ((flags & IOMAP_NOWAIT) && !(flags & IOMAP_REPORT)) {
+ error = -EAGAIN;
+ goto out_unlock;
+ }
/* Trim the mapping to the nearest shared extent boundary. */
error = xfs_reflink_trim_around_shared(ip, &imap, &shared,
&trimmed);
@@ -1049,6 +1054,10 @@ xfs_file_iomap_begin(
}
if ((flags & IOMAP_WRITE) && imap_needs_alloc(inode, &imap, nimaps)) {
+ if (flags & IOMAP_NOWAIT) {
+ error = -EAGAIN;
+ goto out_unlock;
+ }
/*
* We cap the maximum length we map here to MAX_WRITEBACK_PAGES
* pages to keep the chunks of work done where somewhat symmetric
--
2.10.2
^ permalink raw reply related
* [PATCH 6/8] nowait aio: ext4
From: Goldwyn Rodrigues @ 2017-02-28 23:36 UTC (permalink / raw)
To: jack
Cc: hch, linux-fsdevel, linux-block, linux-btrfs, linux-ext4,
linux-xfs, Goldwyn Rodrigues
In-Reply-To: <20170228233610.25456-1-rgoldwyn@suse.de>
From: Goldwyn Rodrigues <rgoldwyn@suse.com>
Return EAGAIN if any of the following checks fail for direct I/O:
+ i_rwsem is lockable
+ Writing beyond end of file (will trigger allocation)
+ Blocks are not allocated at the write location
Signed-off-by: Goldwyn Rodrigues <rgoldwyn@suse.com>
---
fs/ext4/file.c | 53 +++++++++++++++++++++++++++++++++++------------------
1 file changed, 35 insertions(+), 18 deletions(-)
diff --git a/fs/ext4/file.c b/fs/ext4/file.c
index d663d3d..391e03b 100644
--- a/fs/ext4/file.c
+++ b/fs/ext4/file.c
@@ -124,27 +124,22 @@ ext4_unaligned_aio(struct inode *inode, struct iov_iter *from, loff_t pos)
return 0;
}
-/* Is IO overwriting allocated and initialized blocks? */
-static bool ext4_overwrite_io(struct inode *inode, loff_t pos, loff_t len)
+/* Are IO blocks allocated */
+static bool ext4_blocks_mapped(struct inode *inode, loff_t pos, loff_t len,
+ struct ext4_map_blocks *map)
{
- struct ext4_map_blocks map;
unsigned int blkbits = inode->i_blkbits;
int err, blklen;
if (pos + len > i_size_read(inode))
return false;
- map.m_lblk = pos >> blkbits;
- map.m_len = EXT4_MAX_BLOCKS(len, pos, blkbits);
- blklen = map.m_len;
+ map->m_lblk = pos >> blkbits;
+ map->m_len = EXT4_MAX_BLOCKS(len, pos, blkbits);
+ blklen = map->m_len;
- err = ext4_map_blocks(NULL, inode, &map, 0);
- /*
- * 'err==len' means that all of the blocks have been preallocated,
- * regardless of whether they have been initialized or not. To exclude
- * unwritten extents, we need to check m_flags.
- */
- return err == blklen && (map.m_flags & EXT4_MAP_MAPPED);
+ err = ext4_map_blocks(NULL, inode, map, 0);
+ return err == blklen;
}
static ssize_t ext4_write_checks(struct kiocb *iocb, struct iov_iter *from)
@@ -176,6 +171,7 @@ ext4_dax_write_iter(struct kiocb *iocb, struct iov_iter *from)
struct inode *inode = file_inode(iocb->ki_filp);
ssize_t ret;
bool overwrite = false;
+ struct ext4_map_blocks map;
inode_lock(inode);
ret = ext4_write_checks(iocb, from);
@@ -188,7 +184,9 @@ ext4_dax_write_iter(struct kiocb *iocb, struct iov_iter *from)
if (ret)
goto out;
- if (ext4_overwrite_io(inode, iocb->ki_pos, iov_iter_count(from))) {
+ if (ext4_blocks_mapped(inode, iocb->ki_pos,
+ iov_iter_count(from), &map) &&
+ (map.m_flags & EXT4_MAP_MAPPED)) {
overwrite = true;
downgrade_write(&inode->i_rwsem);
}
@@ -209,6 +207,7 @@ ext4_file_write_iter(struct kiocb *iocb, struct iov_iter *from)
{
struct inode *inode = file_inode(iocb->ki_filp);
int o_direct = iocb->ki_flags & IOCB_DIRECT;
+ int nowait = iocb->ki_flags & IOCB_NOWAIT;
int unaligned_aio = 0;
int overwrite = 0;
ssize_t ret;
@@ -218,7 +217,13 @@ ext4_file_write_iter(struct kiocb *iocb, struct iov_iter *from)
return ext4_dax_write_iter(iocb, from);
#endif
- inode_lock(inode);
+ if (o_direct && nowait) {
+ if (!inode_trylock(inode))
+ return -EAGAIN;
+ } else {
+ inode_lock(inode);
+ }
+
ret = ext4_write_checks(iocb, from);
if (ret <= 0)
goto out;
@@ -237,9 +242,21 @@ ext4_file_write_iter(struct kiocb *iocb, struct iov_iter *from)
iocb->private = &overwrite;
/* Check whether we do a DIO overwrite or not */
- if (o_direct && ext4_should_dioread_nolock(inode) && !unaligned_aio &&
- ext4_overwrite_io(inode, iocb->ki_pos, iov_iter_count(from)))
- overwrite = 1;
+ if (o_direct && !unaligned_aio) {
+ struct ext4_map_blocks map;
+ if (ext4_blocks_mapped(inode, iocb->ki_pos,
+ iov_iter_count(from), &map)) {
+ /* To exclude unwritten extents, we need to check
+ * m_flags.
+ */
+ if (ext4_should_dioread_nolock(inode) &&
+ (map.m_flags & EXT4_MAP_MAPPED))
+ overwrite = 1;
+ } else if (iocb->ki_flags & IOCB_NOWAIT) {
+ ret = -EAGAIN;
+ goto out;
+ }
+ }
ret = __generic_file_write_iter(iocb, from);
inode_unlock(inode);
--
2.10.2
^ permalink raw reply related
* [PATCH 5/8] nowait aio: return on congested block device
From: Goldwyn Rodrigues @ 2017-02-28 23:36 UTC (permalink / raw)
To: jack
Cc: hch, linux-fsdevel, linux-block, linux-btrfs, linux-ext4,
linux-xfs, Goldwyn Rodrigues
In-Reply-To: <20170228233610.25456-1-rgoldwyn@suse.de>
From: Goldwyn Rodrigues <rgoldwyn@suse.com>
A new flag BIO_NOWAIT is introduced to identify bio's
orignating from iocb with IOCB_NOWAIT. This flag indicates
to return immediately if a request cannot be made instead
of retrying.
Signed-off-by: Goldwyn Rodrigues <rgoldwyn@suse.com>
---
block/blk-core.c | 13 +++++++++++--
fs/direct-io.c | 11 +++++++++--
include/linux/blk_types.h | 1 +
3 files changed, 21 insertions(+), 4 deletions(-)
diff --git a/block/blk-core.c b/block/blk-core.c
index 61ba08c..e5cfc50 100644
--- a/block/blk-core.c
+++ b/block/blk-core.c
@@ -1258,6 +1258,11 @@ static struct request *get_request(struct request_queue *q, unsigned int op,
if (!IS_ERR(rq))
return rq;
+ if (bio_flagged(bio, BIO_NOWAIT)) {
+ blk_put_rl(rl);
+ return ERR_PTR(-EAGAIN);
+ }
+
if (!gfpflags_allow_blocking(gfp_mask) || unlikely(blk_queue_dying(q))) {
blk_put_rl(rl);
return rq;
@@ -2018,7 +2023,7 @@ blk_qc_t generic_make_request(struct bio *bio)
do {
struct request_queue *q = bdev_get_queue(bio->bi_bdev);
- if (likely(blk_queue_enter(q, false) == 0)) {
+ if (likely(blk_queue_enter(q, bio_flagged(bio, BIO_NOWAIT)) == 0)) {
ret = q->make_request_fn(q, bio);
blk_queue_exit(q);
@@ -2027,7 +2032,11 @@ blk_qc_t generic_make_request(struct bio *bio)
} else {
struct bio *bio_next = bio_list_pop(current->bio_list);
- bio_io_error(bio);
+ if (unlikely(bio_flagged(bio, BIO_NOWAIT))) {
+ bio->bi_error = -EAGAIN;
+ bio_endio(bio);
+ } else
+ bio_io_error(bio);
bio = bio_next;
}
} while (bio);
diff --git a/fs/direct-io.c b/fs/direct-io.c
index c87bae4..2973df0 100644
--- a/fs/direct-io.c
+++ b/fs/direct-io.c
@@ -386,6 +386,9 @@ dio_bio_alloc(struct dio *dio, struct dio_submit *sdio,
else
bio->bi_end_io = dio_bio_end_io;
+ if (dio->iocb->ki_flags & IOCB_NOWAIT)
+ bio_set_flag(bio, BIO_NOWAIT);
+
sdio->bio = bio;
sdio->logical_offset_in_bio = sdio->cur_page_fs_offset;
}
@@ -480,8 +483,12 @@ static int dio_bio_complete(struct dio *dio, struct bio *bio)
unsigned i;
int err;
- if (bio->bi_error)
- dio->io_error = -EIO;
+ if (bio->bi_error) {
+ if (bio_flagged(bio, BIO_NOWAIT))
+ dio->io_error = bio->bi_error;
+ else
+ dio->io_error = -EIO;
+ }
if (dio->is_async && dio->op == REQ_OP_READ && dio->should_dirty) {
err = bio->bi_error;
diff --git a/include/linux/blk_types.h b/include/linux/blk_types.h
index 519ea2c..1d77e9b 100644
--- a/include/linux/blk_types.h
+++ b/include/linux/blk_types.h
@@ -102,6 +102,7 @@ struct bio {
#define BIO_REFFED 8 /* bio has elevated ->bi_cnt */
#define BIO_THROTTLED 9 /* This bio has already been subjected to
* throttling rules. Don't do it again. */
+#define BIO_NOWAIT 10 /* don't block over blk device congestion */
/*
* Flags starting here get preserved by bio_reset() - this includes
--
2.10.2
^ permalink raw reply related
* [PATCH 4/8] nowait aio: Introduce IOMAP_NOWAIT
From: Goldwyn Rodrigues @ 2017-02-28 23:36 UTC (permalink / raw)
To: jack
Cc: hch, linux-fsdevel, linux-block, linux-btrfs, linux-ext4,
linux-xfs, Goldwyn Rodrigues
In-Reply-To: <20170228233610.25456-1-rgoldwyn@suse.de>
From: Goldwyn Rodrigues <rgoldwyn@suse.com>
IOCB_NOWAIT translates to IOMAP_NOWAIT for iomaps.
This is used by XFS in the XFS patch.
Signed-off-by: Goldwyn Rodrigues <rgoldwyn@suse.com>
---
fs/iomap.c | 2 ++
include/linux/iomap.h | 1 +
2 files changed, 3 insertions(+)
diff --git a/fs/iomap.c b/fs/iomap.c
index a51cb4c..3fb68d2 100644
--- a/fs/iomap.c
+++ b/fs/iomap.c
@@ -883,6 +883,8 @@ iomap_dio_rw(struct kiocb *iocb, struct iov_iter *iter, struct iomap_ops *ops,
} else {
dio->flags |= IOMAP_DIO_WRITE;
flags |= IOMAP_WRITE;
+ if (iocb->ki_flags & IOCB_NOWAIT)
+ flags |= IOMAP_NOWAIT;
}
if (mapping->nrpages) {
diff --git a/include/linux/iomap.h b/include/linux/iomap.h
index a4c94b8..d1c33ef 100644
--- a/include/linux/iomap.h
+++ b/include/linux/iomap.h
@@ -51,6 +51,7 @@ struct iomap {
#define IOMAP_REPORT (1 << 2) /* report extent status, e.g. FIEMAP */
#define IOMAP_FAULT (1 << 3) /* mapping for page fault */
#define IOMAP_DIRECT (1 << 4) /* direct I/O */
+#define IOMAP_NOWAIT (1 << 5) /* Don't wait for writeback */
struct iomap_ops {
/*
--
2.10.2
^ permalink raw reply related
* [PATCH 3/8] nowait aio: return if direct write will trigger writeback
From: Goldwyn Rodrigues @ 2017-02-28 23:36 UTC (permalink / raw)
To: jack
Cc: hch, linux-fsdevel, linux-block, linux-btrfs, linux-ext4,
linux-xfs, Goldwyn Rodrigues
In-Reply-To: <20170228233610.25456-1-rgoldwyn@suse.de>
From: Goldwyn Rodrigues <rgoldwyn@suse.com>
Find out if the write will trigger a wait due to writeback. If yes,
return -EAGAIN.
This introduces a new function filemap_range_has_page() which
returns true if the file's mapping has a page within the range
mentioned.
Return -EINVAL for buffered AIO: there are multiple causes of
delay such as page locks, dirty throttling logic, page loading
from disk etc. which cannot be taken care of.
Signed-off-by: Goldwyn Rodrigues <rgoldwyn@suse.com>
---
include/linux/fs.h | 2 ++
mm/filemap.c | 50 +++++++++++++++++++++++++++++++++++++++++++++++---
2 files changed, 49 insertions(+), 3 deletions(-)
diff --git a/include/linux/fs.h b/include/linux/fs.h
index ab2f556..527ef53 100644
--- a/include/linux/fs.h
+++ b/include/linux/fs.h
@@ -2494,6 +2494,8 @@ extern int filemap_fdatawait(struct address_space *);
extern void filemap_fdatawait_keep_errors(struct address_space *);
extern int filemap_fdatawait_range(struct address_space *, loff_t lstart,
loff_t lend);
+extern int filemap_range_has_page(struct address_space *, loff_t lstart,
+ loff_t lend);
extern int filemap_write_and_wait(struct address_space *mapping);
extern int filemap_write_and_wait_range(struct address_space *mapping,
loff_t lstart, loff_t lend);
diff --git a/mm/filemap.c b/mm/filemap.c
index 78dd50e..82335f4 100644
--- a/mm/filemap.c
+++ b/mm/filemap.c
@@ -375,6 +375,39 @@ int filemap_flush(struct address_space *mapping)
}
EXPORT_SYMBOL(filemap_flush);
+/**
+ * filemap_range_has_page - check if a page exists in range.
+ * @mapping: address space structure to wait for
+ * @start_byte: offset in bytes where the range starts
+ * @end_byte: offset in bytes where the range ends (inclusive)
+ *
+ * Find at least one page in the range supplied, usually used to check if
+ * direct writing in this range will trigger a writeback.
+ */
+int filemap_range_has_page(struct address_space *mapping,
+ loff_t start_byte, loff_t end_byte)
+{
+ pgoff_t index = start_byte >> PAGE_SHIFT;
+ pgoff_t end = end_byte >> PAGE_SHIFT;
+ struct pagevec pvec;
+ int ret;
+
+ if (end_byte < start_byte)
+ return 0;
+
+ if (mapping->nrpages == 0)
+ return 0;
+
+ pagevec_init(&pvec, 0);
+ ret = pagevec_lookup(&pvec, mapping, index, 1);
+ if (!ret)
+ return 0;
+ ret = (pvec.pages[0]->index <= end);
+ pagevec_release(&pvec);
+ return ret;
+}
+EXPORT_SYMBOL(filemap_range_has_page);
+
static int __filemap_fdatawait_range(struct address_space *mapping,
loff_t start_byte, loff_t end_byte)
{
@@ -2631,6 +2664,9 @@ inline ssize_t generic_write_checks(struct kiocb *iocb, struct iov_iter *from)
pos = iocb->ki_pos;
+ if ((iocb->ki_flags & IOCB_NOWAIT) && !(iocb->ki_flags & IOCB_DIRECT))
+ return -EINVAL;
+
if (limit != RLIM_INFINITY) {
if (iocb->ki_pos >= limit) {
send_sig(SIGXFSZ, current, 0);
@@ -2700,9 +2736,17 @@ generic_file_direct_write(struct kiocb *iocb, struct iov_iter *from)
write_len = iov_iter_count(from);
end = (pos + write_len - 1) >> PAGE_SHIFT;
- written = filemap_write_and_wait_range(mapping, pos, pos + write_len - 1);
- if (written)
- goto out;
+ if (iocb->ki_flags & IOCB_NOWAIT) {
+ /* If there are pages to writeback, return */
+ if (filemap_range_has_page(inode->i_mapping, pos,
+ pos + iov_iter_count(from)))
+ return -EAGAIN;
+ } else {
+ written = filemap_write_and_wait_range(mapping, pos,
+ pos + write_len - 1);
+ if (written)
+ goto out;
+ }
/*
* After a write we want buffered reads to be sure to go to disk to get
--
2.10.2
^ permalink raw reply related
* [PATCH 2/8] nowait aio: Return if cannot get hold of i_rwsem
From: Goldwyn Rodrigues @ 2017-02-28 23:36 UTC (permalink / raw)
To: jack
Cc: hch, linux-fsdevel, linux-block, linux-btrfs, linux-ext4,
linux-xfs, Goldwyn Rodrigues
In-Reply-To: <20170228233610.25456-1-rgoldwyn@suse.de>
From: Goldwyn Rodrigues <rgoldwyn@suse.com>
A failure to lock i_rwsem would mean there is I/O being performed
by another thread. So, let's bail.
Signed-off-by: Goldwyn Rodrigues <rgoldwyn@suse.com>
---
mm/filemap.c | 7 ++++++-
1 file changed, 6 insertions(+), 1 deletion(-)
diff --git a/mm/filemap.c b/mm/filemap.c
index 3f9afde..78dd50e 100644
--- a/mm/filemap.c
+++ b/mm/filemap.c
@@ -2973,7 +2973,12 @@ ssize_t generic_file_write_iter(struct kiocb *iocb, struct iov_iter *from)
struct inode *inode = file->f_mapping->host;
ssize_t ret;
- inode_lock(inode);
+ if (!inode_trylock(inode)) {
+ /* Don't sleep on inode rwsem */
+ if (iocb->ki_flags & IOCB_NOWAIT)
+ return -EAGAIN;
+ inode_lock(inode);
+ }
ret = generic_write_checks(iocb, from);
if (ret > 0)
ret = __generic_file_write_iter(iocb, from);
--
2.10.2
^ permalink raw reply related
* [PATCH 1/8] nowait aio: Introduce IOCB_FLAG_NOWAIT
From: Goldwyn Rodrigues @ 2017-02-28 23:36 UTC (permalink / raw)
To: jack
Cc: hch, linux-fsdevel, linux-block, linux-btrfs, linux-ext4,
linux-xfs, Goldwyn Rodrigues
In-Reply-To: <20170228233610.25456-1-rgoldwyn@suse.de>
From: Goldwyn Rodrigues <rgoldwyn@suse.com>
This flag informs kernel to bail out if an AIO request will block
for reasons such as file allocations, or a writeback triggered,
or would block while allocating requests while performing
direct I/O.
IOCB_FLAG_NOWAIT is translated to IOCB_NOWAIT for
iocb->ki_flags.
Signed-off-by: Goldwyn Rodrigues <rgoldwyn@suse.com>
---
fs/aio.c | 3 +++
include/linux/fs.h | 1 +
include/uapi/linux/aio_abi.h | 3 +++
3 files changed, 7 insertions(+)
diff --git a/fs/aio.c b/fs/aio.c
index 873b4ca..5ae19ba 100644
--- a/fs/aio.c
+++ b/fs/aio.c
@@ -1586,6 +1586,9 @@ static int io_submit_one(struct kioctx *ctx, struct iocb __user *user_iocb,
req->common.ki_flags |= IOCB_EVENTFD;
}
+ if (iocb->aio_flags & IOCB_FLAG_NOWAIT)
+ req->common.ki_flags |= IOCB_NOWAIT;
+
ret = put_user(KIOCB_KEY, &user_iocb->aio_key);
if (unlikely(ret)) {
pr_debug("EFAULT: aio_key\n");
diff --git a/include/linux/fs.h b/include/linux/fs.h
index 2ba0743..ab2f556 100644
--- a/include/linux/fs.h
+++ b/include/linux/fs.h
@@ -270,6 +270,7 @@ struct writeback_control;
#define IOCB_DSYNC (1 << 4)
#define IOCB_SYNC (1 << 5)
#define IOCB_WRITE (1 << 6)
+#define IOCB_NOWAIT (1 << 7)
struct kiocb {
struct file *ki_filp;
diff --git a/include/uapi/linux/aio_abi.h b/include/uapi/linux/aio_abi.h
index bb2554f..82d1d94 100644
--- a/include/uapi/linux/aio_abi.h
+++ b/include/uapi/linux/aio_abi.h
@@ -51,8 +51,11 @@ enum {
*
* IOCB_FLAG_RESFD - Set if the "aio_resfd" member of the "struct iocb"
* is valid.
+ * IOCB_FLAG_NOWAIT - Set if the user wants the iocb to fail if it would block
+ * for operations such as disk allocation.
*/
#define IOCB_FLAG_RESFD (1 << 0)
+#define IOCB_FLAG_NOWAIT (1 << 1)
/* read() from /dev/aio returns these structures. */
struct io_event {
--
2.10.2
^ permalink raw reply related
* [PATCH 0/8 v2] Non-blocking AIO
From: Goldwyn Rodrigues @ 2017-02-28 23:36 UTC (permalink / raw)
To: jack; +Cc: hch, linux-fsdevel, linux-block, linux-btrfs, linux-ext4,
linux-xfs
This series adds nonblocking feature to asynchronous I/O writes.
io_submit() can be delayed because of a number of reason:
- Block allocation for files
- Data writebacks for direct I/O
- Sleeping because of waiting to acquire i_rwsem
- Congested block device
The goal of the patch series is to return -EAGAIN/-EWOULDBLOCK if
any of these conditions are met. This way userspace can push most
of the write()s to the kernel to the best of its ability to complete
and if it returns -EAGAIN, can defer it to another thread.
In order to enable this, IOCB_FLAG_NOWAIT is introduced in
uapi/linux/aio_abi.h which translates to IOCB_NOWAIT for struct iocb,
BIO_NOWAIT for bio and IOMAP_NOWAIT for iomap.
This feature is provided for direct I/O of asynchronous I/O only. I have
tested it against xfs, ext4, and btrfs.
Changes since v1:
+ Forwardported from 4.9.10
+ changed name from _NONBLOCKING to *_NOWAIT
+ filemap_range_has_page call moved to closer to (just before) calling filemap_write_and_wait_range().
+ BIO_NOWAIT limited to get_request()
+ XFS fixes
- included reflink
- use of xfs_ilock_nowait() instead of a XFS_IOLOCK_NONBLOCKING flag
- Translate the flag through IOMAP_NOWAIT (iomap) to check for
block allocation for the file.
+ ext4 coding style
--
Goldwyn
^ permalink raw reply
* Re: [PATCH v2 05/13] md: raid1: simplify r1buf_pool_free()
From: Shaohua Li @ 2017-02-28 23:31 UTC (permalink / raw)
To: Ming Lei; +Cc: Jens Axboe, linux-raid, linux-block, Christoph Hellwig
In-Reply-To: <1488296503-4987-6-git-send-email-tom.leiming@gmail.com>
On Tue, Feb 28, 2017 at 11:41:35PM +0800, Ming Lei wrote:
> This patch gets each page's reference of each bio for resync,
> then r1buf_pool_free() gets simplified a lot.
>
> The same policy has been taken in raid10's buf pool allocation/free
> too.
We are going to delete the code, this simplify isn't really required.
>
> Signed-off-by: Ming Lei <tom.leiming@gmail.com>
> ---
> drivers/md/raid1.c | 15 +++++++--------
> 1 file changed, 7 insertions(+), 8 deletions(-)
>
> diff --git a/drivers/md/raid1.c b/drivers/md/raid1.c
> index 25c9172db639..c442b4657e2f 100644
> --- a/drivers/md/raid1.c
> +++ b/drivers/md/raid1.c
> @@ -139,9 +139,12 @@ static void * r1buf_pool_alloc(gfp_t gfp_flags, void *data)
> /* If not user-requests, copy the page pointers to all bios */
> if (!test_bit(MD_RECOVERY_REQUESTED, &pi->mddev->recovery)) {
> for (i=0; i<RESYNC_PAGES ; i++)
> - for (j=1; j<pi->raid_disks; j++)
> - r1_bio->bios[j]->bi_io_vec[i].bv_page =
> + for (j=1; j<pi->raid_disks; j++) {
> + struct page *page =
> r1_bio->bios[0]->bi_io_vec[i].bv_page;
> + get_page(page);
> + r1_bio->bios[j]->bi_io_vec[i].bv_page = page;
> + }
> }
>
> r1_bio->master_bio = NULL;
> @@ -166,12 +169,8 @@ static void r1buf_pool_free(void *__r1_bio, void *data)
> struct r1bio *r1bio = __r1_bio;
>
> for (i = 0; i < RESYNC_PAGES; i++)
> - for (j = pi->raid_disks; j-- ;) {
> - if (j == 0 ||
> - r1bio->bios[j]->bi_io_vec[i].bv_page !=
> - r1bio->bios[0]->bi_io_vec[i].bv_page)
> - safe_put_page(r1bio->bios[j]->bi_io_vec[i].bv_page);
> - }
> + for (j = pi->raid_disks; j-- ;)
> + safe_put_page(r1bio->bios[j]->bi_io_vec[i].bv_page);
> for (i=0 ; i < pi->raid_disks; i++)
> bio_put(r1bio->bios[i]);
>
> --
> 2.7.4
>
^ permalink raw reply
* Re: [RFC] failure atomic writes for file systems and block devices
From: Darrick J. Wong @ 2017-02-28 23:22 UTC (permalink / raw)
To: Christoph Hellwig; +Cc: linux-fsdevel, linux-xfs, linux-block
In-Reply-To: <20170228145737.19016-1-hch@lst.de>
On Tue, Feb 28, 2017 at 06:57:25AM -0800, Christoph Hellwig wrote:
> Hi all,
>
> this series implements a new O_ATOMIC flag for failure atomic writes
> to files. It is based on and tries to unify to earlier proposals,
> the first one for block devices by Chris Mason:
>
> https://lwn.net/Articles/573092/
>
> and the second one for regular files, published by HP Research at
> Usenix FAST 2015:
>
> https://www.usenix.org/conference/fast15/technical-sessions/presentation/verma
>
> It adds a new O_ATOMIC flag for open, which requests writes to be
> failure-atomic, that is either the whole write makes it to persistent
> storage, or none of it, even in case of power of other failures.
>
> There are two implementation various of this: on block devices O_ATOMIC
> must be combined with O_(D)SYNC so that storage devices that can handle
> large writes atomically can simply do that without any additional work.
> This case is supported by NVMe.
>
> The second case is for file systems, where we simply write new blocks
> out of places and then remap them into the file atomically on either
> completion of an O_(D)SYNC write or when fsync is called explicitly.
>
> The semantics of the latter case are explained in detail at the Usenix
> paper above.
(Assuming there's no syncv involved here...?)
> Last but not least a new fcntl is implemented to provide information
> about I/O restrictions such as alignment requirements and the maximum
> atomic write size.
>
> The implementation is simple and clean, but I'm rather unhappy about
> the interface as it has too many failure modes to bullet proof. For
> one old kernels ignore unknown open flags silently, so applications
Ok, heh, disregard my review comment (for the xfs part) about the
seemingly insufficient O_ATOMIC validation.
> have to check the F_IOINFO fcntl before, which is a bit of a killer.
> Because of that I've also not implemented any other validity checks
> yet, as they might make thing even worse when an open on a not supported
> file system or device fails, but not on an old kernel. Maybe we need
> a new open version that checks arguments properly first?
Does fcntl(F_SETFL...) suffer from this?
> Also I'm really worried about the NVMe failure modes - devices simply
> advertise an atomic write size, with no way for the device to know
> that the host requested a given write to be atomic, and thus no
> error reporting.
Yikes!
> This is made worse by NVMe 1.2 adding per-namespace
> atomic I/O parameters that devices can use to introduce additional
> odd alignment quirks - while there is some language in the spec
> requiring them not to weaken the per-controller guarantees it all
> looks rather weak and I'm not too confident in all implementations
> getting everything right.
>
> Last but not least this depends on a few XFS patches, so to actually
> apply / run the patches please use this git tree:
Well, the XFS parts don't look too bad....
--D
>
> git://git.infradead.org/users/hch/vfs.git O_ATOMIC
>
> Gitweb:
>
> http://git.infradead.org/users/hch/vfs.git/shortlog/refs/heads/O_ATOMIC
^ permalink raw reply
* Re: [PATCH] blkcg: allocate struct blkcg_gq outside request queue spinlock
From: Tejun Heo @ 2017-02-28 22:47 UTC (permalink / raw)
To: Tahsin Erdogan; +Cc: Jens Axboe, linux-block, David Rientjes, linux-kernel
In-Reply-To: <20170228024957.4314-1-tahsin@google.com>
Hello,
Overall, the approach looks good to me but please see below.
On Mon, Feb 27, 2017 at 06:49:57PM -0800, Tahsin Erdogan wrote:
> @@ -806,44 +807,99 @@ int blkg_conf_prep(struct blkcg *blkcg, const struct blkcg_policy *pol,
> if (!disk)
> return -ENODEV;
> if (part) {
> - owner = disk->fops->owner;
> - put_disk(disk);
> - module_put(owner);
> - return -ENODEV;
> + ret = -ENODEV;
> + goto fail;
> + }
> +
> + q = disk->queue;
> +
> + if (!blkcg_policy_enabled(q, pol)) {
> + ret = -EOPNOTSUPP;
> + goto fail;
Pulling this out of the queue_lock doesn't seem safe to me. This
function may end up calling into callbacks of disabled policies this
way.
> + /*
> + * Create blkgs walking down from blkcg_root to @blkcg, so that all
> + * non-root blkgs have access to their parents.
> + */
> + while (true) {
> + struct blkcg *pos = blkcg;
> + struct blkcg *parent;
> + struct blkcg_gq *new_blkg;
> +
> + parent = blkcg_parent(blkcg);
> + while (parent && !__blkg_lookup(parent, q, false)) {
> + pos = parent;
> + parent = blkcg_parent(parent);
> + }
Hmm... how about adding @new_blkg to blkg_lookup_create() and calling
it with non-NULL @new_blkg until it succeeds? Wouldn't that be
simpler?
> +
> + new_blkg = blkg_alloc(pos, q, GFP_KERNEL);
> + if (unlikely(!new_blkg)) {
> + ret = -ENOMEM;
> + goto fail;
> + }
> +
> + rcu_read_lock();
> + spin_lock_irq(q->queue_lock);
> +
> + /* Lookup again since we dropped the lock for blkg_alloc(). */
> + blkg = __blkg_lookup(pos, q, false);
> + if (blkg) {
> + blkg_free(new_blkg);
> + } else {
> + blkg = blkg_create(pos, q, new_blkg);
> + if (unlikely(IS_ERR(blkg))) {
> + ret = PTR_ERR(blkg);
> + goto fail_unlock;
> + }
than duplicating the same logic here?
Thanks.
--
tejun
^ permalink raw reply
* Re: [RFC] failure atomic writes for file systems and block devices
From: Chris Mason @ 2017-02-28 20:48 UTC (permalink / raw)
To: Christoph Hellwig, linux-fsdevel, linux-xfs, linux-block
In-Reply-To: <20170228145737.19016-1-hch@lst.de>
On 02/28/2017 09:57 AM, Christoph Hellwig wrote:
> Hi all,
>
> this series implements a new O_ATOMIC flag for failure atomic writes
> to files. It is based on and tries to unify to earlier proposals,
> the first one for block devices by Chris Mason:
>
> https://urldefense.proofpoint.com/v2/url?u=https-3A__lwn.net_Articles_573092_&d=DwIBAg&c=5VD0RTtNlTh3ycd41b3MUw&r=9QPtTAxcitoznaWRKKHoEQ&m=P5byIhbDCF-kdlNpZVpxMKG3E36-cQ-lK27coqUFUng&s=rqXtuRMvf2rijHel_VAiO-KQ8AtQ5DXEI2obnCI_ljQ&e=
>
> and the second one for regular files, published by HP Research at
> Usenix FAST 2015:
>
> https://urldefense.proofpoint.com/v2/url?u=https-3A__www.usenix.org_conference_fast15_technical-2Dsessions_presentation_verma&d=DwIBAg&c=5VD0RTtNlTh3ycd41b3MUw&r=9QPtTAxcitoznaWRKKHoEQ&m=P5byIhbDCF-kdlNpZVpxMKG3E36-cQ-lK27coqUFUng&s=ilnrrNs8nG4_UV2xx7tc2Efm20d2Wa8PHoJE8WUTCwI&e=
>
> It adds a new O_ATOMIC flag for open, which requests writes to be
> failure-atomic, that is either the whole write makes it to persistent
> storage, or none of it, even in case of power of other failures.
>
> There are two implementation various of this: on block devices O_ATOMIC
> must be combined with O_(D)SYNC so that storage devices that can handle
> large writes atomically can simply do that without any additional work.
> This case is supported by NVMe.
>
Hi Christoph,
This is great, and supporting code in both dio and bio get rid of some
of the warts from when I tried. The DIO_PAGES define used to be an
upper limit on the max contiguous bio that would get built, but that's
much better now.
One thing that isn't clear to me is how we're dealing with boundary bio
mappings, which will get submitted by submit_page_section()
sdio->boundary = buffer_boundary(map_bh);
In btrfs I'd just chain things together and do the extent pointer swap
afterwards, but I didn't follow the XFS code well enough to see how its
handled there. But either way it feels like an error prone surprise
waiting for later, and one gap we really want to get right in the FS
support is O_ATOMIC across a fragmented extent.
If I'm reading the XFS patches right, the code always cows for atomic.
Are you planning on adding an optimization to use atomic support in the
device to skip COW when possible?
To turn off mysql double buffering, we only need 16K or 64K writes,
which most of the time you'd be able to pass down directly without cows.
-chris
^ permalink raw reply
* Re: [Lsf-pc] [LSF/MM TOPIC] do we really need PG_error at all?
From: NeilBrown @ 2017-02-28 20:45 UTC (permalink / raw)
To: Jeff Layton, Andreas Dilger
Cc: linux-block, linux-scsi, lsf-pc, Neil Brown, LKML,
James Bottomley, linux-mm, linux-fsdevel
In-Reply-To: <1488244308.7627.5.camel@redhat.com>
[-- Attachment #1: Type: text/plain, Size: 3827 bytes --]
On Mon, Feb 27 2017, Jeff Layton wrote:
> On Tue, 2017-02-28 at 10:32 +1100, NeilBrown wrote:
>> On Mon, Feb 27 2017, Andreas Dilger wrote:
>>
>> >
>> > My thought is that PG_error is definitely useful for applications to get
>> > correct errors back when doing write()/sync_file_range() so that they know
>> > there is an error in the data that _they_ wrote, rather than receiving an
>> > error for data that may have been written by another thread, and in turn
>> > clearing the error from another thread so it *doesn't* know it had a write
>> > error.
>>
>> It might be useful in that way, but it is not currently used that way.
>> Such usage would be a change in visible behaviour.
>>
>> sync_file_range() calls filemap_fdatawait_range(), which calls
>> filemap_check_errors().
>> If there have been any errors in the file recently, inside or outside
>> the range, the latter will return an error which will propagate up.
>>
>> >
>> > As for stray sync() clearing PG_error from underneath an application, that
>> > shouldn't happen since filemap_fdatawait_keep_errors() doesn't clear errors
>> > and is used by device flushing code (fdatawait_one_bdev(), wait_sb_inodes()).
>>
>> filemap_fdatawait_keep_errors() calls __filemap_fdatawait_range() which
>> clears PG_error on every page.
>> What it doesn't do is call filemap_check_errors(), and so doesn't clear
>> AS_ENOSPC or AS_EIO.
>>
>>
>
> I think it's helpful to get a clear idea of what happens now in the face
> of errors and what we expect to happen, and I don't quite have that yet:
>
> --------------------------8<-----------------------------
> void page_endio(struct page *page, bool is_write, int err)
> {
> if (!is_write) {
> if (!err) {
> SetPageUptodate(page);
> } else {
> ClearPageUptodate(page);
> SetPageError(page);
> }
> unlock_page(page);
> } else {
> if (err) {
> SetPageError(page);
> if (page->mapping)
> mapping_set_error(page->mapping, err);
> }
> end_page_writeback(page);
> }
> }
> --------------------------8<-----------------------------
>
> ...not everything uses page_endio, but it's a good place to look since
> we have both flavors of error handling in one place.
>
> On a write error, we SetPageError and set the error in the mapping.
>
> What I'm not clear on is:
>
> 1) what happens to the page at that point when we get a writeback error?
> Does it just remain in-core and is allowed to service reads (assuming
> that it was uptodate before)?
Yes, it remains in core and can service reads. It is no different from
a page on which a write recent succeeded, except that the write didn't
succeed so the contents of backing store might be different from the
contents of the page.
>
> Can I redirty it and have it retry the write? Is there standard behavior
> for this or is it just up to the whim of the filesystem?
Everything is at the whim of the filesystem, but I doubt if many differ
from the above.
NeilBrown
>
> I'll probably have questions about the read side as well, but for now it
> looks like it's mostly used in an ad-hoc way to communicate errors
> across subsystems (block to fs layer, for instance).
> --
> Jeff Layton <jlayton@redhat.com>
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 832 bytes --]
^ permalink raw reply
* Re: [PATCH] sbitmap: boundary checks for resized bitmap
From: Omar Sandoval @ 2017-02-28 19:15 UTC (permalink / raw)
To: Hannes Reinecke; +Cc: Jens Axboe, Omar Sandoval, linux-block, Hannes Reinecke
In-Reply-To: <1487157042-5621-1-git-send-email-hare@suse.de>
On Wed, Feb 15, 2017 at 12:10:42PM +0100, Hannes Reinecke wrote:
> If the sbitmap gets resized we need to ensure not to overflow
> the original allocation. And we should limit the search in
> sbitmap_any_bit_set() to the available depth to avoid looking
> into unused space.
Hey, Hannes, I don't really like this change. It's easy enough for the
caller to keep track of this and check themselves if they really care. I
even included a comment in sbitmap.h to that effect:
/**
* sbitmap_resize() - Resize a &struct sbitmap.
* @sb: Bitmap to resize.
* @depth: New number of bits to resize to.
*
* Doesn't reallocate anything. It's up to the caller to ensure that the new
* depth doesn't exceed the depth that the sb was initialized with.
*/
As for the sbitmap_any_bit_set() change, the bits beyond the actual
depth should all be zero, so I don't think that change is worth it,
either.
Thanks!
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox