Re: [Cluster-devel] [PATCH 04/21] fs: Replace CURRENT_TIME with current_fs_time() for inode timestamps
for nilfs2 bits: Acked-by: Ryusuke Konishi Thanks, Ryusuke Konishi
Re: [Cluster-devel] [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
Re: [Cluster-devel] [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
Re: [Cluster-devel] [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
Re: [Cluster-devel] [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
Re: [Cluster-devel] [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
Re: [Cluster-devel] [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
Re: [Cluster-devel] [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
Re: [Cluster-devel] [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
>
Re: [Cluster-devel] [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
Re: [Cluster-devel] [PATCH 9/9] mm: return an ERR_PTR from __filemap_get_folio
On Wed, Jan 18, 2023 at 7:41 PM Christoph Hellwig wrote: > > Instead of returning NULL for all errors, distinguish between: > > - no entry found and not asked to allocated (-ENOENT) > - failed to allocate memory (-ENOMEM) > - would block (-EAGAIN) > > so that callers don't have to guess the error based on the passed > in flags. > > Also pass through the error through the direct callers: > filemap_get_folio, filemap_lock_folio filemap_grab_folio > and filemap_get_incore_folio. As for the comments describing the return values of these callers, isn't it necessary to rewrite the value from NULL in case of errors ? Regards, Ryusuke Konishi
Re: [Cluster-devel] [PATCH 7/7] mm: return an ERR_PTR from __filemap_get_folio
On Sat, Jan 21, 2023 at 3:59 PM Christoph Hellwig wrote: > > Instead of returning NULL for all errors, distinguish between: > > - no entry found and not asked to allocated (-ENOENT) > - failed to allocate memory (-ENOMEM) > - would block (-EAGAIN) > > so that callers don't have to guess the error based on the passed > in flags. > > Also pass through the error through the direct callers: > filemap_get_folio, filemap_lock_folio filemap_grab_folio > and filemap_get_incore_folio. > > Signed-off-by: Christoph Hellwig > --- For > fs/nilfs2/page.c | 6 +++--- Acked-by: Ryusuke Konishi Thanks, Ryusuke Konishi
