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