Re: [PATCH RFC v3 for-6.8/block 04/17] mtd: block2mtd: use bdev apis
Hi, 在 2024/01/05 14:10, Christoph Hellwig 写道: On Thu, Jan 04, 2024 at 12:28:55PM +0100, Jan Kara wrote: What do you think? Because when we are working with the folios it is rather natural to use their mapping for dirty balancing? The real problem is that block2mtd pokes way to deep into block internals. I think the saviour here is Christians series to replace the bdev handle with a struct file, which will allow to use the normal file write path here and get rid of the entire layering volation. Yes, looks like lots of patches from this set is not needed anymore. I'll stop sending v4 and just send some patches that is not related to 'bd_inode' separately. Thanks, Kuai .
Re: [PATCH RFC v3 for-6.8/block 04/17] mtd: block2mtd: use bdev apis
On Thu, Jan 04, 2024 at 12:28:55PM +0100, Jan Kara wrote: > What do you think? Because when we are working with the folios it is rather > natural to use their mapping for dirty balancing? The real problem is that block2mtd pokes way to deep into block internals. I think the saviour here is Christians series to replace the bdev handle with a struct file, which will allow to use the normal file write path here and get rid of the entire layering volation.
Re: [PATCH RFC v3 for-6.8/block 04/17] mtd: block2mtd: use bdev apis
Hi,
在 2024/01/04 19:28, Jan Kara 写道:
On Thu 21-12-23 16:56:59, Yu Kuai wrote:
From: Yu Kuai
On the one hand covert to use folio while reading bdev inode, on the
other hand prevent to access bd_inode directly.
Signed-off-by: Yu Kuai
...
+ for (p = folio_address(folio); p < max; p++)
if (*p != -1UL) {
- lock_page(page);
- memset(page_address(page), 0xff, PAGE_SIZE);
- set_page_dirty(page);
- unlock_page(page);
- balance_dirty_pages_ratelimited(mapping);
+ folio_lock(folio);
+ memset(folio_address(folio), 0xff,
+ folio_size(folio));
+ folio_mark_dirty(folio);
+ folio_unlock(folio);
+ bdev_balance_dirty_pages_ratelimited(bdev);
Rather then creating this bdev_balance_dirty_pages_ratelimited() just for
MTD perhaps we can have here (and in other functions):
...
mapping = folio_mapping(folio);
folio_unlock(folio);
if (mapping)
balance_dirty_pages_ratelimited(mapping);
What do you think? Because when we are working with the folios it is rather
natural to use their mapping for dirty balancing?
I think this is a great idea! And bdev_balance_dirty_pages_ratelimited()
can be removed as well.
Thanks,
Kuai
Honza
Re: [PATCH RFC v3 for-6.8/block 04/17] mtd: block2mtd: use bdev apis
On Thu 21-12-23 16:56:59, Yu Kuai wrote:
> From: Yu Kuai
>
> On the one hand covert to use folio while reading bdev inode, on the
> other hand prevent to access bd_inode directly.
>
> Signed-off-by: Yu Kuai
...
> + for (p = folio_address(folio); p < max; p++)
> if (*p != -1UL) {
> - lock_page(page);
> - memset(page_address(page), 0xff, PAGE_SIZE);
> - set_page_dirty(page);
> - unlock_page(page);
> - balance_dirty_pages_ratelimited(mapping);
> + folio_lock(folio);
> + memset(folio_address(folio), 0xff,
> +folio_size(folio));
> + folio_mark_dirty(folio);
> + folio_unlock(folio);
> + bdev_balance_dirty_pages_ratelimited(bdev);
Rather then creating this bdev_balance_dirty_pages_ratelimited() just for
MTD perhaps we can have here (and in other functions):
...
mapping = folio_mapping(folio);
folio_unlock(folio);
if (mapping)
balance_dirty_pages_ratelimited(mapping);
What do you think? Because when we are working with the folios it is rather
natural to use their mapping for dirty balancing?
Honza
--
Jan Kara
SUSE Labs, CR
[PATCH RFC v3 for-6.8/block 04/17] mtd: block2mtd: use bdev apis
From: Yu Kuai
On the one hand covert to use folio while reading bdev inode, on the
other hand prevent to access bd_inode directly.
Signed-off-by: Yu Kuai
---
drivers/mtd/devices/block2mtd.c | 81 +++--
1 file changed, 36 insertions(+), 45 deletions(-)
diff --git a/drivers/mtd/devices/block2mtd.c b/drivers/mtd/devices/block2mtd.c
index aa44a23ec045..cf201bf73184 100644
--- a/drivers/mtd/devices/block2mtd.c
+++ b/drivers/mtd/devices/block2mtd.c
@@ -46,40 +46,34 @@ struct block2mtd_dev {
/* Static info about the MTD, used in cleanup_module */
static LIST_HEAD(blkmtd_device_list);
-
-static struct page *page_read(struct address_space *mapping, pgoff_t index)
-{
- return read_mapping_page(mapping, index, NULL);
-}
-
/* erase a specified part of the device */
static int _block2mtd_erase(struct block2mtd_dev *dev, loff_t to, size_t len)
{
- struct address_space *mapping =
- dev->bdev_handle->bdev->bd_inode->i_mapping;
- struct page *page;
+ struct block_device *bdev = dev->bdev_handle->bdev;
+ struct folio *folio;
pgoff_t index = to >> PAGE_SHIFT; // page index
int pages = len >> PAGE_SHIFT;
u_long *p;
u_long *max;
while (pages) {
- page = page_read(mapping, index);
- if (IS_ERR(page))
- return PTR_ERR(page);
+ folio = bdev_read_folio(bdev, index << PAGE_SHIFT);
+ if (IS_ERR(folio))
+ return PTR_ERR(folio);
- max = page_address(page) + PAGE_SIZE;
- for (p=page_address(page); ppriv;
- struct address_space *mapping =
- dev->bdev_handle->bdev->bd_inode->i_mapping;
- struct page *page;
+ struct folio *folio;
pgoff_t index = from >> PAGE_SHIFT;
int offset = from & (PAGE_SIZE-1);
int cpylen;
@@ -120,12 +112,13 @@ static int block2mtd_read(struct mtd_info *mtd, loff_t
from, size_t len,
cpylen = len; // this page
len = len - cpylen;
- page = page_read(mapping, index);
- if (IS_ERR(page))
- return PTR_ERR(page);
+ folio = bdev_read_folio(dev->bdev_handle->bdev,
+ index << PAGE_SHIFT);
+ if (IS_ERR(folio))
+ return PTR_ERR(folio);
- memcpy(buf, page_address(page) + offset, cpylen);
- put_page(page);
+ memcpy(buf, folio_address(folio) + offset, cpylen);
+ folio_put(folio);
if (retlen)
*retlen += cpylen;
@@ -141,9 +134,8 @@ static int block2mtd_read(struct mtd_info *mtd, loff_t
from, size_t len,
static int _block2mtd_write(struct block2mtd_dev *dev, const u_char *buf,
loff_t to, size_t len, size_t *retlen)
{
- struct page *page;
- struct address_space *mapping =
- dev->bdev_handle->bdev->bd_inode->i_mapping;
+ struct block_device *bdev = dev->bdev_handle->bdev;
+ struct folio *folio;
pgoff_t index = to >> PAGE_SHIFT; // page index
int offset = to & ~PAGE_MASK; // page offset
int cpylen;
@@ -155,18 +147,18 @@ static int _block2mtd_write(struct block2mtd_dev *dev,
const u_char *buf,
cpylen = len; // this page
len = len - cpylen;
- page = page_read(mapping, index);
- if (IS_ERR(page))
- return PTR_ERR(page);
+ folio = bdev_read_folio(bdev, index << PAGE_SHIFT);
+ if (IS_ERR(folio))
+ return PTR_ERR(folio);
- if (memcmp(page_address(page)+offset, buf, cpylen)) {
- lock_page(page);
- memcpy(page_address(page) + offset, buf, cpylen);
- set_page_dirty(page);
- unlock_page(page);
- balance_dirty_pages_ratelimited(mapping);
+ if (memcmp(folio_address(folio) + offset, buf, cpylen)) {
+ folio_lock(folio);
+ memcpy(folio_address(folio) + offset, buf, cpylen);
+ folio_mark_dirty(folio);
+ folio_unlock(folio);
+ bdev_balance_dirty_pages_ratelimited(bdev);
}
- put_page(page);
+ folio_put(folio);
if (retlen)
*retlen += cpylen;
@@ -211,8 +203,7 @@ static void block2mtd_free_device(struct block2mtd_dev *dev)
kfree(dev->mtd.name);
if (dev->bdev_handle) {
- invalidate_mapping_pages(
- dev->bdev_handle->bdev->bd_inode->i_mapping, 0, -1);
+ invalidate_bdev(dev->bdev_h
