Re: [f2fs-dev] [PATCH 02/12] nilfs2: use setup_bdev_super to de-duplicate the mount code
On Thu, Aug 10, 2023 at 8:05 PM Jan Kara wrote:
>
> On Fri 04-08-23 11:01:39, Ryusuke Konishi wrote:
> > On Thu, Aug 3, 2023 at 8:46 PM Jan Kara wrote:
> > >
> > > On Wed 02-08-23 17:41:21, Christoph Hellwig wrote:
> > > > Use the generic setup_bdev_super helper to open the main block device
> > > > and do various bits of superblock setup instead of duplicating the
> > > > logic. This includes moving to the new scheme implemented in common
> > > > code that only opens the block device after the superblock has
> > > > allocated.
> > > >
> > > > It does not yet convert nilfs2 to the new mount API, but doing so will
> > > > become a bit simpler after this first step.
> > > >
> > > > Signed-off-by: Christoph Hellwig
> > >
> > > AFAICS nilfs2 could *almost* use mount_bdev() directly and then just do
> > > its
> >
> > > snapshot thing after mount_bdev() returns. But it has this weird logic
> > > that: "if the superblock is already mounted but we can shrink the whole
> > > dcache, then do remount instead of ignoring mount options". Firstly, this
> > > looks racy - what prevents someone from say opening a file on the sb just
> > > after nilfs_tree_is_busy() shrinks dcache? Secondly, it is inconsistent
> > > with any other filesystem so it's going to surprise sysadmins not
> > > intimately knowing nilfs2. Thirdly, from userspace you cannot tell what
> > > your mount call is going to do. Last but not least, what is it really good
> > > for? Ryusuke, can you explain please?
> > >
> > > Honza
> >
> > I think you are referring to the following part:
> >
> > >if (!s->s_root) {
> > ...
> > >} else if (!sd.cno) {
> > >if (nilfs_tree_is_busy(s->s_root)) {
> > >if ((flags ^ s->s_flags) & SB_RDONLY) {
> > >nilfs_err(s,
> > > "the device already has a %s
> > > mount.",
> > > sb_rdonly(s) ? "read-only" :
> > > "read/write");
> > >err = -EBUSY;
> > >goto failed_super;
> > >}
> > >} else {
> > >/*
> > > * Try remount to setup mount states if the current
> > > * tree is not mounted and only snapshots use this
> > > sb.
> > > */
> > >err = nilfs_remount(s, &flags, data);
> > >if (err)
> > >goto failed_super;
> > >}
> > >}
> >
> > What this logic is trying to do is, if there is already a nilfs2 mount
> > instance for the device, and are trying to mounting the current tree
> > (sd.cno is 0, so this is not a snapshot mount), then will switch
> > depending on whether the current tree has a mount:
> >
> > - If the current tree is mounted, it's just like a normal filesystem.
> > (A read-only mount and a read/write mount can't coexist, so check
> > that, and reuse the instance if possible)
> > - Otherwise, i.e. for snapshot mounts only, do whatever is necessary
> > to add a new current mount, such as starting a log writer.
> >Since it does the same thing that nilfs_remount does, so
> > nilfs_remount() is used there.
> >
> > Whether or not there is a current tree mount can be determined by
> > d_count(s->s_root) > 1 as nilfs_tree_is_busy() does.
> > Where s->s_root is always the root dentry of the current tree, not
> > that of the mounted snapshot.
>
> I see now, thanks for explanation! But one thing still is not clear to me.
> If you say have a snapshot mounted read-write and then you mount the
> current snapshot (cno == 0) read-only, you'll switch the whole superblock
> to read-only state. So also the mounted snapshot is suddently read-only
> which is unexpected and actually supposedly breaks things because you can
> still have file handles open for writing on the snapshot etc.. So how do
> you solve that?
>
> Honza
One thing I have to tell you as a premise is that nilfs2's snapshot
mounts (cno !=
Re: [f2fs-dev] [PATCH 02/12] nilfs2: use setup_bdev_super to de-duplicate the mount code
On Thu, Aug 3, 2023 at 12:41 AM Christoph Hellwig wrote: > > Use the generic setup_bdev_super helper to open the main block device > and do various bits of superblock setup instead of duplicating the > logic. This includes moving to the new scheme implemented in common > code that only opens the block device after the superblock has allocated. > > It does not yet convert nilfs2 to the new mount API, but doing so will > become a bit simpler after this first step. > > Signed-off-by: Christoph Hellwig Acked-by: Ryusuke Konishi This patch itself looks to properly convert nilfs_mount etc. Thank you so much. Regards, Ryusuke Konishi ___ Linux-f2fs-devel mailing list Linux-f2fs-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel
Re: [f2fs-dev] [PATCH 02/12] nilfs2: use setup_bdev_super to de-duplicate the mount code
On Thu, Aug 3, 2023 at 8:46 PM Jan Kara wrote:
>
> On Wed 02-08-23 17:41:21, Christoph Hellwig wrote:
> > Use the generic setup_bdev_super helper to open the main block device
> > and do various bits of superblock setup instead of duplicating the
> > logic. This includes moving to the new scheme implemented in common
> > code that only opens the block device after the superblock has allocated.
> >
> > It does not yet convert nilfs2 to the new mount API, but doing so will
> > become a bit simpler after this first step.
> >
> > Signed-off-by: Christoph Hellwig
>
> AFAICS nilfs2 could *almost* use mount_bdev() directly and then just do its
> snapshot thing after mount_bdev() returns. But it has this weird logic
> that: "if the superblock is already mounted but we can shrink the whole
> dcache, then do remount instead of ignoring mount options". Firstly, this
> looks racy - what prevents someone from say opening a file on the sb just
> after nilfs_tree_is_busy() shrinks dcache? Secondly, it is inconsistent
> with any other filesystem so it's going to surprise sysadmins not
> intimately knowing nilfs2. Thirdly, from userspace you cannot tell what
> your mount call is going to do. Last but not least, what is it really good
> for? Ryusuke, can you explain please?
>
> Honza
I think you are referring to the following part:
>if (!s->s_root) {
...
>} else if (!sd.cno) {
>if (nilfs_tree_is_busy(s->s_root)) {
>if ((flags ^ s->s_flags) & SB_RDONLY) {
>nilfs_err(s,
> "the device already has a %s mount.",
> sb_rdonly(s) ? "read-only" :
> "read/write");
>err = -EBUSY;
>goto failed_super;
>}
>} else {
>/*
> * Try remount to setup mount states if the current
> * tree is not mounted and only snapshots use this sb.
> */
>err = nilfs_remount(s, &flags, data);
>if (err)
>goto failed_super;
>}
>}
What this logic is trying to do is, if there is already a nilfs2 mount
instance for the device, and are trying to mounting the current tree
(sd.cno is 0, so this is not a snapshot mount), then will switch
depending on whether the current tree has a mount:
- If the current tree is mounted, it's just like a normal filesystem.
(A read-only mount and a read/write mount can't coexist, so check
that, and reuse the instance if possible)
- Otherwise, i.e. for snapshot mounts only, do whatever is necessary
to add a new current mount, such as starting a log writer.
Since it does the same thing that nilfs_remount does, so
nilfs_remount() is used there.
Whether or not there is a current tree mount can be determined by
d_count(s->s_root) > 1 as nilfs_tree_is_busy() does.
Where s->s_root is always the root dentry of the current tree, not
that of the mounted snapshot.
I remember that calling shrink_dcache_parent() before this test was to
do the test correctly if there was garbage left in the dcache from the
past current mount.
If the current tree isn't mounted, it just cleans up the garbage, and
the reference count wouldn't have incremented in parallel.
If the current tree is mounted, d_count(s->s_root) will not decrease
to 1, so it's not a problem.
However, this will cause unexpected dcache shrinkage for the in-use
tree, so it's not a good idea, as you pointed out. If there is
another way of judging without this side effect, it should be
replaced.
I will reply here once.
Regards,
Ryusuke Konishi
___
Linux-f2fs-devel mailing list
Linux-f2fs-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel
Re: [f2fs-dev] [PATCH v2 18/23] nilfs2: Convert nilfs_lookup_dirty_data_buffers() to use filemap_get_folios_tag()
On Tue, Sep 13, 2022 at 3:30 AM Vishal Moola (Oracle) wrote: > > Convert function to use folios throughout. This is in preparation for > the removal of find_get_pages_range_tag(). > > Signed-off-by: Vishal Moola (Oracle) > --- > fs/nilfs2/segment.c | 29 - > 1 file changed, 16 insertions(+), 13 deletions(-) Acked-by: Ryusuke Konishi Looks good. Thank you for reflecting the previous comment. Ryusuke Konishi ___ Linux-f2fs-devel mailing list Linux-f2fs-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel
Re: [f2fs-dev] [PATCH 21/23] nilfs2: Convert nilfs_copy_dirty_pages() to use filemap_get_folios_tag()
On Fri, Sep 2, 2022 at 7:18 AM Vishal Moola (Oracle) wrote: > > Convert function to use folios throughout. This is in preparation for > the removal of find_get_pages_range_tag(). > > Signed-off-by: Vishal Moola (Oracle) > --- > fs/nilfs2/page.c | 39 --- > 1 file changed, 20 insertions(+), 19 deletions(-) Acked-by: Ryusuke Konishi Thanks, Ryusuke Konishi ___ Linux-f2fs-devel mailing list Linux-f2fs-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel
Re: [f2fs-dev] [PATCH 18/23] nilfs2: Convert nilfs_lookup_dirty_data_buffers() to use filemap_get_folios_tag()
On Fri, Sep 2, 2022 at 7:07 AM Vishal Moola (Oracle) wrote:
>
> Convert function to use folios throughout. This is in preparation for
> the removal of find_get_pages_range_tag().
>
> Signed-off-by: Vishal Moola (Oracle)
> ---
> fs/nilfs2/segment.c | 29 -
> 1 file changed, 16 insertions(+), 13 deletions(-)
>
> diff --git a/fs/nilfs2/segment.c b/fs/nilfs2/segment.c
> index 0afe0832c754..e95c667bdc8f 100644
> --- a/fs/nilfs2/segment.c
> +++ b/fs/nilfs2/segment.c
> @@ -680,7 +680,7 @@ static size_t nilfs_lookup_dirty_data_buffers(struct
> inode *inode,
> loff_t start, loff_t end)
> {
> struct address_space *mapping = inode->i_mapping;
> - struct pagevec pvec;
> + struct folio_batch fbatch;
> pgoff_t index = 0, last = ULONG_MAX;
> size_t ndirties = 0;
> int i;
> @@ -694,23 +694,26 @@ static size_t nilfs_lookup_dirty_data_buffers(struct
> inode *inode,
> index = start >> PAGE_SHIFT;
> last = end >> PAGE_SHIFT;
> }
> - pagevec_init(&pvec);
> + folio_batch_init(&fbatch);
> repeat:
> if (unlikely(index > last) ||
> - !pagevec_lookup_range_tag(&pvec, mapping, &index, last,
> - PAGECACHE_TAG_DIRTY))
> + !filemap_get_folios_tag(mapping, &index, last,
> + PAGECACHE_TAG_DIRTY, &fbatch))
> return ndirties;
>
> - for (i = 0; i < pagevec_count(&pvec); i++) {
> + for (i = 0; i < folio_batch_count(&fbatch); i++) {
> struct buffer_head *bh, *head;
> - struct page *page = pvec.pages[i];
> + struct folio *folio = fbatch.folios[i];
>
> - lock_page(page);
> - if (!page_has_buffers(page))
> - create_empty_buffers(page, i_blocksize(inode), 0);
> - unlock_page(page);
> + head = folio_buffers(folio);
> + folio_lock(folio);
Could you please swap these two lines to keep the "head" check in the lock?
Thanks,
Ryusuke Konishi
> + if (!head) {
> + create_empty_buffers(&folio->page,
> i_blocksize(inode), 0);
> + head = folio_buffers(folio);
> + }
> + folio_unlock(folio);
>
> - bh = head = page_buffers(page);
> + bh = head;
> do {
> if (!buffer_dirty(bh) || buffer_async_write(bh))
> continue;
> @@ -718,13 +721,13 @@ static size_t nilfs_lookup_dirty_data_buffers(struct
> inode *inode,
> list_add_tail(&bh->b_assoc_buffers, listp);
> ndirties++;
> if (unlikely(ndirties >= nlimit)) {
> - pagevec_release(&pvec);
> + folio_batch_release(&fbatch);
> cond_resched();
> return ndirties;
> }
> } while (bh = bh->b_this_page, bh != head);
> }
> - pagevec_release(&pvec);
> + folio_batch_release(&fbatch);
> cond_resched();
> goto repeat;
> }
> --
> 2.36.1
>
___
Linux-f2fs-devel mailing list
Linux-f2fs-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel
Re: [f2fs-dev] [PATCH 22/23] nilfs2: Convert nilfs_clear_dirty_pages() to use filemap_get_folios_tag()
On Fri, Sep 2, 2022 at 7:14 AM Vishal Moola (Oracle) wrote: > > Convert function to use folios throughout. This is in preparation for > the removal of find_get_pages_range_tag(). > > Signed-off-by: Vishal Moola (Oracle) > --- > fs/nilfs2/page.c | 20 ++-- > 1 file changed, 10 insertions(+), 10 deletions(-) Acked-by: Ryusuke Konishi Thanks, Ryusuke Konishi ___ Linux-f2fs-devel mailing list Linux-f2fs-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel
Re: [f2fs-dev] [PATCH 19/23] nilfs2: Convert nilfs_lookup_dirty_node_buffers() to use filemap_get_folios_tag()
On Fri, Sep 2, 2022 at 7:07 AM Vishal Moola (Oracle) wrote: > > Convert function to use folios throughout. This is in preparation for > the removal of find_get_pages_range_tag(). > > Signed-off-by: Vishal Moola (Oracle) > --- > fs/nilfs2/segment.c | 15 +++ > 1 file changed, 7 insertions(+), 8 deletions(-) Acked-by: Ryusuke Konishi Thanks, Ryusuke Konishi ___ Linux-f2fs-devel mailing list Linux-f2fs-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel
Re: [f2fs-dev] [PATCH 20/23] nilfs2: Convert nilfs_btree_lookup_dirty_buffers() to use filemap_get_folios_tag()
On Fri, Sep 2, 2022 at 7:06 AM Vishal Moola (Oracle) wrote: > > Convert function to use folios throughout. This is in preparation for > the removal of find_get_pages_range_tag(). > > Signed-off-by: Vishal Moola (Oracle) > --- > fs/nilfs2/btree.c | 14 +++--- > 1 file changed, 7 insertions(+), 7 deletions(-) Acked-by: Ryusuke Konishi Thanks, Ryusuke Konishi ___ Linux-f2fs-devel mailing list Linux-f2fs-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel
Re: [f2fs-dev] [PATCH 07/10] nilfs2: Convert nilfs_copy_back_pages() to use filemap_get_folios()
On Mon, Jun 6, 2022 at 1:00 PM Matthew Wilcox (Oracle) wrote: > > Use folios throughout. > > Signed-off-by: Matthew Wilcox (Oracle) > --- > fs/nilfs2/page.c | 60 > 1 file changed, 30 insertions(+), 30 deletions(-) Throughout, looks good to me. Thank you so much for your great work. Acked-by: Ryusuke Konishi Ryusuke Konishi ___ Linux-f2fs-devel mailing list Linux-f2fs-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel
Re: [f2fs-dev] [PATCH 26/27] block: uncouple REQ_OP_SECURE_ERASE from REQ_OP_DISCARD
ine blk_queue_add_random(q)test_bit(QUEUE_FLAG_ADD_RANDOM,
> &(q)->queue_flags)
> #define blk_queue_zone_resetall(q) \
> test_bit(QUEUE_FLAG_ZONE_RESETALL, &(q)->queue_flags)
> -#define blk_queue_secure_erase(q) \
> - (test_bit(QUEUE_FLAG_SECERASE, &(q)->queue_flags))
> #define blk_queue_dax(q) test_bit(QUEUE_FLAG_DAX, &(q)->queue_flags)
> #define blk_queue_pci_p2pdma(q)\
> test_bit(QUEUE_FLAG_PCI_P2PDMA, &(q)->queue_flags)
> @@ -947,6 +945,8 @@ extern void blk_queue_chunk_sectors(struct request_queue
> *, unsigned int);
> extern void blk_queue_max_segments(struct request_queue *, unsigned short);
> extern void blk_queue_max_discard_segments(struct request_queue *,
> unsigned short);
> +void blk_queue_max_secure_erase_sectors(struct request_queue *q,
> + unsigned int max_sectors);
> extern void blk_queue_max_segment_size(struct request_queue *, unsigned int);
> extern void blk_queue_max_discard_sectors(struct request_queue *q,
> unsigned int max_discard_sectors);
> @@ -1087,13 +1087,12 @@ static inline long nr_blockdev_pages(void)
>
> extern void blk_io_schedule(void);
>
> -#define BLKDEV_DISCARD_SECURE (1 << 0)/* issue a secure erase */
> -
> -extern int blkdev_issue_discard(struct block_device *bdev, sector_t sector,
> - sector_t nr_sects, gfp_t gfp_mask, unsigned long flags);
> -extern int __blkdev_issue_discard(struct block_device *bdev, sector_t sector,
> - sector_t nr_sects, gfp_t gfp_mask, int flags,
> - struct bio **biop);
> +int blkdev_issue_discard(struct block_device *bdev, sector_t sector,
> + sector_t nr_sects, gfp_t gfp_mask);
> +int __blkdev_issue_discard(struct block_device *bdev, sector_t sector,
> + sector_t nr_sects, gfp_t gfp_mask, struct bio **biop);
> +int blkdev_issue_secure_erase(struct block_device *bdev, sector_t sector,
> + sector_t nr_sects, gfp_t gfp);
>
> #define BLKDEV_ZERO_NOUNMAP(1 << 0) /* do not free blocks */
> #define BLKDEV_ZERO_NOFALLBACK (1 << 1) /* don't write explicit zeroes */
> @@ -1112,7 +,7 @@ static inline int sb_issue_discard(struct super_block
> *sb, sector_t block,
> SECTOR_SHIFT),
> nr_blocks << (sb->s_blocksize_bits -
> SECTOR_SHIFT),
> - gfp_mask, flags);
> + gfp_mask);
> }
> static inline int sb_issue_zeroout(struct super_block *sb, sector_t block,
> sector_t nr_blocks, gfp_t gfp_mask)
> @@ -1262,6 +1261,12 @@ static inline unsigned int
> bdev_discard_granularity(struct block_device *bdev)
> return bdev_get_queue(bdev)->limits.discard_granularity;
> }
>
> +static inline unsigned int
> +bdev_max_secure_erase_sectors(struct block_device *bdev)
> +{
> + return bdev_get_queue(bdev)->limits.max_secure_erase_sectors;
> +}
> +
> static inline unsigned int bdev_write_zeroes_sectors(struct block_device
> *bdev)
> {
> struct request_queue *q = bdev_get_queue(bdev);
> diff --git a/mm/swapfile.c b/mm/swapfile.c
> index 5d9cedf9e7b84..a2b31fea0c42e 100644
> --- a/mm/swapfile.c
> +++ b/mm/swapfile.c
> @@ -179,7 +179,7 @@ static int discard_swap(struct swap_info_struct *si)
> nr_blocks = ((sector_t)se->nr_pages - 1) << (PAGE_SHIFT - 9);
> if (nr_blocks) {
> err = blkdev_issue_discard(si->bdev, start_block,
> - nr_blocks, GFP_KERNEL, 0);
> + nr_blocks, GFP_KERNEL);
> if (err)
> return err;
> cond_resched();
> @@ -190,7 +190,7 @@ static int discard_swap(struct swap_info_struct *si)
> nr_blocks = (sector_t)se->nr_pages << (PAGE_SHIFT - 9);
>
> err = blkdev_issue_discard(si->bdev, start_block,
> - nr_blocks, GFP_KERNEL, 0);
> + nr_blocks, GFP_KERNEL);
> if (err)
> break;
>
> @@ -254,7 +254,7 @@ static void discard_swap_cluster(struct swap_info_struct
> *si,
> start_block <<= PAGE_SHIFT - 9;
> nr_blocks <<= PAGE_SHIFT - 9;
> if (blkdev_issue_discard(si->bdev, start_block,
> - nr_blocks, GFP_NOIO, 0))
> + nr_blocks, GFP_NOIO))
> break;
>
> se = next_se(se);
> --
> 2.30.2
>
For nilfs2 pieces,
Acked-by: Ryusuke Konishi
Thanks,
Ryusuke Konishi
___
Linux-f2fs-devel mailing list
Linux-f2fs-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel
Re: [f2fs-dev] [PATCH 24/27] block: add a bdev_discard_granularity helper
ong arg)
> if (copy_from_user(&range, argp, sizeof(range)))
> return -EFAULT;
>
> - range.minlen = max_t(u64, q->limits.discard_granularity,
> + range.minlen = max_t(u64,
> bdev_discard_granularity(sb->s_bdev),
> range.minlen);
> ret = ocfs2_trim_fs(sb, &range);
> if (ret < 0)
> diff --git a/fs/xfs/xfs_discard.c b/fs/xfs/xfs_discard.c
> index a4e6609d616b7..e2ada115c23f9 100644
> --- a/fs/xfs/xfs_discard.c
> +++ b/fs/xfs/xfs_discard.c
> @@ -152,8 +152,8 @@ xfs_ioc_trim(
> struct xfs_mount*mp,
> struct fstrim_range __user *urange)
> {
> - struct request_queue*q =
> bdev_get_queue(mp->m_ddev_targp->bt_bdev);
> - unsigned intgranularity = q->limits.discard_granularity;
> + unsigned intgranularity =
> + bdev_discard_granularity(mp->m_ddev_targp->bt_bdev);
> struct fstrim_range range;
> xfs_daddr_t start, end, minlen;
> xfs_agnumber_t start_agno, end_agno, agno;
> diff --git a/include/linux/blkdev.h b/include/linux/blkdev.h
> index ce16247d3afab..7b9c0cf95d2d5 100644
> --- a/include/linux/blkdev.h
> +++ b/include/linux/blkdev.h
> @@ -1259,6 +1259,11 @@ static inline unsigned int
> bdev_max_discard_sectors(struct block_device *bdev)
> return bdev_get_queue(bdev)->limits.max_discard_sectors;
> }
>
> +static inline unsigned int bdev_discard_granularity(struct block_device
> *bdev)
> +{
> + return bdev_get_queue(bdev)->limits.discard_granularity;
> +}
> +
> static inline unsigned int bdev_write_zeroes_sectors(struct block_device
> *bdev)
> {
> struct request_queue *q = bdev_get_queue(bdev);
> --
> 2.30.2
>
I got the following checkpatch warning:
WARNING: 'retreive' may be misspelled - perhaps 'retrieve'?
#101:
block_device based helper to retreive the discard granularity.
total: 0 errors, 1 warnings, 294 lines checked
The changes themselves look good.
Acked-by: Ryusuke Konishi
Thanks,
Ryusuke Konishi
___
Linux-f2fs-devel mailing list
Linux-f2fs-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel
Re: [f2fs-dev] [PATCH 23/27] block: add a bdev_max_discard_sectors helper
On Wed, Apr 6, 2022 at 11:05 PM Christoph Hellwig wrote: > > Add a helper to query the number of sectors support per each discard bio > based on the block device and use this helper to stop various places from > poking into the request_queue to see if discard is supported and if so how > much. This mirrors what is done e.g. for write zeroes as well. > > Signed-off-by: Christoph Hellwig > --- ... > diff --git a/drivers/target/target_core_device.c > b/drivers/target/target_core_device.c > index 16e775bcf4a7c..7d510e4231713 100644 > --- a/drivers/target/target_core_device.c > +++ b/drivers/target/target_core_device.c > @@ -829,9 +829,7 @@ struct se_device *target_alloc_device(struct se_hba *hba, > const char *name) > } > > /* > - * Check if the underlying struct block_device request_queue supports > - * the QUEUE_FLAG_DISCARD bit for UNMAP/WRITE_SAME in SCSI + TRIM > - * in ATA and we need to set TPE=1 > + * Check if the underlying struct block_device request_queue supports disard. > */ Here was a typo: s/disard/discard/ On Thu, Apr 7, 2022 at 12:19 AM Andreas Gruenbacher wrote: > If I'm misreading things, could you please document that > bdev_max_discard_sectors() != 0 implies that discard is supported? I got the same impression. Checking the discard support with bdev_max_discard_sectors() != 0 seems a bit unclear than before. Thanks, Ryusuke Konishi ___ Linux-f2fs-devel mailing list Linux-f2fs-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel
Re: [f2fs-dev] [PATCH 08/11] Remove bdi_congested() and wb_congested() and related functions
On Thu, Feb 10, 2022 at 2:41 PM NeilBrown wrote:
>
> These functions are no longer useful as no BDIs report congestions any
> more.
>
> Removing the test on bdi_write_contested() in current_may_throttle()
> could cause a small change in behaviour, but only when PF_LOCAL_THROTTLE
> is set.
>
> So replace the calls by 'false' and simplify the code - and remove the
> functions.
>
> Acked-by: Ryusuke Konishi (for nilfs bits)
> Signed-off-by: NeilBrown
> ---
> drivers/block/drbd/drbd_int.h |3 ---
> drivers/block/drbd/drbd_req.c |3 +--
> fs/ext2/ialloc.c |5 -
> fs/nilfs2/segbuf.c| 15 ---
> fs/xfs/xfs_buf.c |3 ---
> include/linux/backing-dev.h | 26 --
> mm/vmscan.c |4 +---
> 7 files changed, 2 insertions(+), 57 deletions(-)
>
> diff --git a/drivers/block/drbd/drbd_int.h b/drivers/block/drbd/drbd_int.h
> index f27d5b0f9a0b..f804b1bfb3e6 100644
> --- a/drivers/block/drbd/drbd_int.h
> +++ b/drivers/block/drbd/drbd_int.h
> @@ -638,9 +638,6 @@ enum {
> STATE_SENT, /* Do not change state/UUIDs while this is
> set */
> CALLBACK_PENDING, /* Whether we have a call_usermodehelper(,
> UMH_WAIT_PROC)
> * pending, from drbd worker context.
> -* If set, bdi_write_congested() returns true,
> -* so shrink_page_list() would not recurse
> into,
> -* and potentially deadlock on, this drbd
> worker.
> */
> DISCONNECT_SENT,
>
> diff --git a/drivers/block/drbd/drbd_req.c b/drivers/block/drbd/drbd_req.c
> index 3235532ae077..2e5fb7e442e3 100644
> --- a/drivers/block/drbd/drbd_req.c
> +++ b/drivers/block/drbd/drbd_req.c
> @@ -909,8 +909,7 @@ static bool remote_due_to_read_balancing(struct
> drbd_device *device, sector_t se
>
> switch (rbm) {
> case RB_CONGESTED_REMOTE:
> - return bdi_read_congested(
> - device->ldev->backing_bdev->bd_disk->bdi);
> + return 0;
> case RB_LEAST_PENDING:
> return atomic_read(&device->local_cnt) >
> atomic_read(&device->ap_pending_cnt) +
> atomic_read(&device->rs_pending_cnt);
> diff --git a/fs/ext2/ialloc.c b/fs/ext2/ialloc.c
> index df14e750e9fe..998dd2ac8008 100644
> --- a/fs/ext2/ialloc.c
> +++ b/fs/ext2/ialloc.c
> @@ -170,11 +170,6 @@ static void ext2_preread_inode(struct inode *inode)
> unsigned long offset;
> unsigned long block;
> struct ext2_group_desc * gdp;
> - struct backing_dev_info *bdi;
> -
> - bdi = inode_to_bdi(inode);
> - if (bdi_rw_congested(bdi))
> - return;
>
> block_group = (inode->i_ino - 1) / EXT2_INODES_PER_GROUP(inode->i_sb);
> gdp = ext2_get_group_desc(inode->i_sb, block_group, NULL);
> diff --git a/fs/nilfs2/segbuf.c b/fs/nilfs2/segbuf.c
> index 43287b0d3e9b..c4510f79037f 100644
> --- a/fs/nilfs2/segbuf.c
> +++ b/fs/nilfs2/segbuf.c
> @@ -343,17 +343,6 @@ static int nilfs_segbuf_submit_bio(struct
> nilfs_segment_buffer *segbuf,
> struct bio *bio = wi->bio;
> int err;
>
> - if (segbuf->sb_nbio > 0 &&
> - bdi_write_congested(segbuf->sb_super->s_bdi)) {
> - wait_for_completion(&segbuf->sb_bio_event);
> - segbuf->sb_nbio--;
> - if (unlikely(atomic_read(&segbuf->sb_err))) {
> - bio_put(bio);
> - err = -EIO;
> - goto failed;
> - }
> - }
> -
> bio->bi_end_io = nilfs_end_bio_write;
> bio->bi_private = segbuf;
> bio_set_op_attrs(bio, mode, mode_flags);
> @@ -365,10 +354,6 @@ static int nilfs_segbuf_submit_bio(struct
> nilfs_segment_buffer *segbuf,
> wi->nr_vecs = min(wi->max_pages, wi->rest_blocks);
> wi->start = wi->end;
> return 0;
> -
> - failed:
> - wi->bio = NULL;
> - return err;
> }
In this revised version, "int err" is no longer used, so could you
delete it as well ?
Regards,
Ryusuke Konishi
>
> /**
> diff --git a/fs/xfs/xfs_buf.c b/fs/xfs/xfs_buf.c
> index b45e0d50a405..b7ebcfe6b8d3 100644
> --- a/fs/xfs/xfs_buf.c
> +++ b/fs/xfs/xfs_buf.c
> @@ -843,9 +843,6 @@ xfs_buf_readahead_map(
> {
> struct xfs_buf *bp;
>
> - if (bdi_read_congested(t
Re: [f2fs-dev] [PATCH 2/9] Remove bdi_congested() and wb_congested() and related functions
On Thu, Jan 27, 2022 at 11:47 AM NeilBrown wrote: > > These functions are no longer useful as the only bdis that report > congestion are in ceph, fuse, and nfs. None of those bdis can be the > target of the calls in drbd, ext2, nilfs2, or xfs. > > Removing the test on bdi_write_contested() in current_may_throttle() > could cause a small change in behaviour, but only when PF_LOCAL_THROTTLE > is set. > > So replace the calls by 'false' and simplify the code - and remove the > functions. > > Signed-off-by: NeilBrown > --- > drivers/block/drbd/drbd_int.h |3 --- > drivers/block/drbd/drbd_req.c |3 +-- > fs/ext2/ialloc.c |2 -- > fs/nilfs2/segbuf.c| 11 --- > fs/xfs/xfs_buf.c |3 --- > include/linux/backing-dev.h | 26 -- > mm/vmscan.c |4 +--- > 7 files changed, 2 insertions(+), 50 deletions(-) for nilfs2 bits, Acked-by: Ryusuke Konishi Thanks, Ryusuke Konishi ___ Linux-f2fs-devel mailing list Linux-f2fs-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel
Re: [f2fs-dev] [PATCH 04/21] fs: Replace CURRENT_TIME with current_fs_time() for inode timestamps
for nilfs2 bits: Acked-by: Ryusuke Konishi Thanks, Ryusuke Konishi -- What NetFlow Analyzer can do for you? Monitors network bandwidth and traffic patterns at an interface-level. Reveals which users, apps, and protocols are consuming the most bandwidth. Provides multi-vendor support for NetFlow, J-Flow, sFlow and other flows. Make informed decisions using capacity planning reports. https://ad.doubleclick.net/ddm/clk/305295220;132659582;e ___ Linux-f2fs-devel mailing list Linux-f2fs-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel
Re: [f2fs-dev] [dm-devel] [PATCH 04/42] fs: have submit_bh users pass in op and flags separately
On Fri, 15 Apr 2016 05:39:24 -0500, mchri...@redhat.com wrote:
> From: Mike Christie
>
> This has submit_bh users pass in the operation and flags separately,
> so submit_bh_wbc can setup bio->bi_op and bio-bi_rw on the bio that
> is submitted.
>
> Signed-off-by: Mike Christie
> Reviewed-by: Christoph Hellwig
> Reviewed-by: Hannes Reinecke
> ---
Looks good with regard to nilfs2.
Acked-by: Ryusuke Konishi
> drivers/md/bitmap.c | 4 ++--
> fs/btrfs/check-integrity.c | 24 ++--
> fs/btrfs/check-integrity.h | 2 +-
> fs/btrfs/disk-io.c | 4 ++--
> fs/buffer.c | 54
> +++--
> fs/ext4/balloc.c| 2 +-
> fs/ext4/ialloc.c| 2 +-
> fs/ext4/inode.c | 2 +-
> fs/ext4/mmp.c | 4 ++--
> fs/fat/misc.c | 2 +-
> fs/gfs2/bmap.c | 2 +-
> fs/gfs2/dir.c | 2 +-
> fs/gfs2/meta_io.c | 6 ++---
> fs/jbd2/commit.c| 6 ++---
> fs/jbd2/journal.c | 8 +++
> fs/nilfs2/btnode.c | 6 ++---
> fs/nilfs2/btnode.h | 2 +-
> fs/nilfs2/btree.c | 6 +++--
> fs/nilfs2/gcinode.c | 5 +++--
> fs/nilfs2/mdt.c | 11 -
> fs/ntfs/aops.c | 6 ++---
> fs/ntfs/compress.c | 2 +-
> fs/ntfs/file.c | 2 +-
> fs/ntfs/logfile.c | 2 +-
> fs/ntfs/mft.c | 4 ++--
> fs/ocfs2/buffer_head_io.c | 8 +++
> fs/reiserfs/inode.c | 4 ++--
> fs/reiserfs/journal.c | 6 ++---
> fs/ufs/util.c | 2 +-
> include/linux/buffer_head.h | 9
> 30 files changed, 103 insertions(+), 96 deletions(-)
>
> diff --git a/drivers/md/bitmap.c b/drivers/md/bitmap.c
> index 3fe86b5..8b2e16f 100644
> --- a/drivers/md/bitmap.c
> +++ b/drivers/md/bitmap.c
> @@ -294,7 +294,7 @@ static void write_page(struct bitmap *bitmap, struct page
> *page, int wait)
> atomic_inc(&bitmap->pending_writes);
> set_buffer_locked(bh);
> set_buffer_mapped(bh);
> - submit_bh(WRITE | REQ_SYNC, bh);
> + submit_bh(REQ_OP_WRITE, REQ_SYNC, bh);
> bh = bh->b_this_page;
> }
>
> @@ -389,7 +389,7 @@ static int read_page(struct file *file, unsigned long
> index,
> atomic_inc(&bitmap->pending_writes);
> set_buffer_locked(bh);
> set_buffer_mapped(bh);
> - submit_bh(READ, bh);
> + submit_bh(REQ_OP_READ, 0, bh);
> }
> block++;
> bh = bh->b_this_page;
> diff --git a/fs/btrfs/check-integrity.c b/fs/btrfs/check-integrity.c
> index 9400acd..f82190f 100644
> --- a/fs/btrfs/check-integrity.c
> +++ b/fs/btrfs/check-integrity.c
> @@ -2856,12 +2856,12 @@ static struct btrfsic_dev_state
> *btrfsic_dev_state_lookup(
> return ds;
> }
>
> -int btrfsic_submit_bh(int rw, struct buffer_head *bh)
> +int btrfsic_submit_bh(int op, int op_flags, struct buffer_head *bh)
> {
> struct btrfsic_dev_state *dev_state;
>
> if (!btrfsic_is_initialized)
> - return submit_bh(rw, bh);
> + return submit_bh(op, op_flags, bh);
>
> mutex_lock(&btrfsic_mutex);
> /* since btrfsic_submit_bh() might also be called before
> @@ -2870,26 +2870,26 @@ int btrfsic_submit_bh(int rw, struct buffer_head *bh)
>
> /* Only called to write the superblock (incl. FLUSH/FUA) */
> if (NULL != dev_state &&
> - (rw & WRITE) && bh->b_size > 0) {
> + (op == REQ_OP_WRITE) && bh->b_size > 0) {
> u64 dev_bytenr;
>
> dev_bytenr = 4096 * bh->b_blocknr;
> if (dev_state->state->print_mask &
> BTRFSIC_PRINT_MASK_SUBMIT_BIO_BH)
> printk(KERN_INFO
> -"submit_bh(rw=0x%x, blocknr=%llu (bytenr %llu),"
> -" size=%zu, data=%p, bdev=%p)\n",
> -rw, (unsigned long long)bh->b_blocknr,
> +"submit_bh(op=0x%x,0x%x, blocknr=%llu "
> +"(bytenr %llu), size=%zu, data=%p, bdev=%p)\n",
> +op, op_flags, (unsigned long long)bh->b_blocknr,
> dev_bytenr, bh->b_size, bh->b_data, bh
Re: [f2fs-dev] [PATCH 16/42] nilfs: set bi_op to REQ_OP
On Fri, 15 Apr 2016 14:15:51 -0500, mchri...@redhat.com wrote:
> From: Mike Christie
>
> This patch has nilfs use bio->bi_op for REQ_OPs and rq_flag_bits
> to bio->bi_rw.
>
> Signed-off-by: Mike Christie
> Reviewed-by: Christoph Hellwig
> Reviewed-by: Hannes Reinecke
> ---
> fs/nilfs2/segbuf.c | 18 ++
> 1 file changed, 10 insertions(+), 8 deletions(-)
Looks good to me.
Acked-by: Ryusuke Konishi
Thanks,
Ryuske Konishi
>
> diff --git a/fs/nilfs2/segbuf.c b/fs/nilfs2/segbuf.c
> index 7666f1d..7b13e14 100644
> --- a/fs/nilfs2/segbuf.c
> +++ b/fs/nilfs2/segbuf.c
> @@ -350,7 +350,8 @@ static void nilfs_end_bio_write(struct bio *bio)
> }
>
> static int nilfs_segbuf_submit_bio(struct nilfs_segment_buffer *segbuf,
> -struct nilfs_write_info *wi, int mode)
> +struct nilfs_write_info *wi, int mode,
> +int mode_flags)
> {
> struct bio *bio = wi->bio;
> int err;
> @@ -368,7 +369,8 @@ static int nilfs_segbuf_submit_bio(struct
> nilfs_segment_buffer *segbuf,
>
> bio->bi_end_io = nilfs_end_bio_write;
> bio->bi_private = segbuf;
> - bio->bi_rw = mode;
> + bio->bi_op = mode;
> + bio->bi_rw = mode_flags;
> submit_bio(bio);
> segbuf->sb_nbio++;
>
> @@ -442,7 +444,7 @@ static int nilfs_segbuf_submit_bh(struct
> nilfs_segment_buffer *segbuf,
> return 0;
> }
> /* bio is FULL */
> - err = nilfs_segbuf_submit_bio(segbuf, wi, mode);
> + err = nilfs_segbuf_submit_bio(segbuf, wi, mode, 0);
> /* never submit current bh */
> if (likely(!err))
> goto repeat;
> @@ -466,19 +468,19 @@ static int nilfs_segbuf_write(struct
> nilfs_segment_buffer *segbuf,
> {
> struct nilfs_write_info wi;
> struct buffer_head *bh;
> - int res = 0, rw = WRITE;
> + int res = 0;
>
> wi.nilfs = nilfs;
> nilfs_segbuf_prepare_write(segbuf, &wi);
>
> list_for_each_entry(bh, &segbuf->sb_segsum_buffers, b_assoc_buffers) {
> - res = nilfs_segbuf_submit_bh(segbuf, &wi, bh, rw);
> + res = nilfs_segbuf_submit_bh(segbuf, &wi, bh, REQ_OP_WRITE);
> if (unlikely(res))
> goto failed_bio;
> }
>
> list_for_each_entry(bh, &segbuf->sb_payload_buffers, b_assoc_buffers) {
> - res = nilfs_segbuf_submit_bh(segbuf, &wi, bh, rw);
> + res = nilfs_segbuf_submit_bh(segbuf, &wi, bh, REQ_OP_WRITE);
> if (unlikely(res))
> goto failed_bio;
> }
> @@ -488,8 +490,8 @@ static int nilfs_segbuf_write(struct nilfs_segment_buffer
> *segbuf,
>* Last BIO is always sent through the following
>* submission.
>*/
> - rw |= REQ_SYNC;
> - res = nilfs_segbuf_submit_bio(segbuf, &wi, rw);
> + res = nilfs_segbuf_submit_bio(segbuf, &wi, REQ_OP_WRITE,
> + REQ_SYNC);
> }
>
> failed_bio:
> --
> 2.7.2
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-fsdevel" in
> the body of a message to majord...@vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
--
Find and fix application performance issues faster with Applications Manager
Applications Manager provides deep performance insights into multiple tiers of
your business applications. It resolves application problems quickly and
reduces your MTTR. Get your free trial!
https://ad.doubleclick.net/ddm/clk/302982198;130105516;z
___
Linux-f2fs-devel mailing list
Linux-f2fs-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel
