Hi Kuai!
On Thu 04-01-24 20:19:05, Yu Kuai wrote:
> 在 2024/01/04 19:06, Jan Kara 写道:
> > On Thu 21-12-23 16:56:57, Yu Kuai wrote:
> > > From: Yu Kuai
> > >
> > > Avoid to access bd_inode directly, prepare to remove bd_inode from
> > > bloc
y ext4), and prepare to
> prevent calling mapping_gfp_constraint() directly on bd_inode->i_mapping
> in ext4.
>
> Signed-off-by: Yu Kuai
I'm not enthusiastic about this but I guess it is as good as it gets
without larger clean
On Thu 21-12-23 16:58:46, Yu Kuai wrote:
> From: Yu Kuai
>
> Avoid to access bd_inode directly, prepare to remove bd_inode from
> block_device.
>
> Signed-off-by: Yu Kuai
Looks good to me. Feel free to add:
Reviewed-by: Jan Kara
But note there are changes pending to
ernal.h
> index b0409badb017..c9206351b485 100644
> --- a/fs/erofs/internal.h
> +++ b/fs/erofs/internal.h
> @@ -224,8 +224,10 @@ enum erofs_kmap_type {
>
> struct erofs_buf {
> struct inode *inode;
> + struct block_device *bdev;
> struct page *page;
> void *base;
> + u8 blkszbits;
> enum erofs_kmap_type kmap_type;
> };
> #define __EROFS_BUF_INITIALIZER ((struct erofs_buf){ .page = NULL })
> --
> 2.39.2
>
--
Jan Kara
SUSE Labs, CR
btrfs developers know and maybe it is just a
cargo cult. And it is not like this would be the first occurence of this
anti-pattern in btrfs - see e.g. device_list_add(), add_missing_dev(),
btrfs_destroy_delalloc_inodes() (here the wrapping around
invalidate_inode_pages2() looks really weird), and many others...
Honza
--
Jan Kara
SUSE Labs, CR
re working with the folios it is rather
natural to use their mapping for dirty balancing?
Honza
--
Jan Kara
SUSE Labs, CR
pages using the ->launder_folio method). So I think you'll
need helper like invalidate_bdev2() for this.
Honza
--
Jan Kara
SUSE Labs, CR
g nobody is keen
on doing ;)
Honza
--
Jan Kara
SUSE Labs, CR
op_flags, gfp, true);
> +}
> +EXPORT_SYMBOL(__bread_gfp2);
__bread_gfp2() is not a great name, why not just using bread_gfp()
directly? I'm not a huge fan of boolean arguments but three different flags
arguments would be too much for my taste ;) so I guess I can live with
that.
Honza
--
Jan Kara
SUSE Labs, CR
ree to add:
Reviewed-by: Jan Kara
Honza
> ---
> fs/ext4/ext4.h| 13 -
> fs/ext4/inode.c | 8
> fs/ext4/super.c | 45 ++---
> fs/ext4/syml
;
> Signed-off-by: Yu Kuai
Agreed, this should not be needed anymore. We'll see whether this is true
also in practice :). Feel free to add:
Reviewed-by: Jan Kara
Honza
--
Jan Kara
SUSE Labs, CR
On Mon 11-12-23 22:08:39, Yu Kuai wrote:
> From: Yu Kuai
>
> Avoid to access bd_inode directly, prepare to remove bd_inode from
> block_devcie.
^^^ device
> Signed-off-by: Yu Kuai
Looks good. Feel free to add:
Review
Honza
--
Jan Kara
SUSE Labs, CR
On Wed 27-09-23 18:21:19, Christian Brauner wrote:
> On Wed, 27 Sep 2023 11:34:07 +0200, Jan Kara wrote:
> > Create struct bdev_handle that contains all parameters that need to be
> > passed to blkdev_put() and provide bdev_open_* functions that return
> > this structure
Convert xen/blkback to use bdev_open_by_dev() and pass the
handle around.
CC: xen-devel@lists.xenproject.org
Acked-by: Christoph Hellwig
Acked-by: Christian Brauner
Signed-off-by: Jan Kara
---
drivers/block/xen-blkback/blkback.c | 4 +--
drivers/block/xen-blkback/common.h | 4 +--
drivers
On Fri 25-08-23 15:32:47, Christian Brauner wrote:
> On Wed, Aug 23, 2023 at 12:48:11PM +0200, Jan Kara wrote:
> > Hello,
> >
> > this is a v3 of the patch series which implements the idea of
> > blkdev_get_by_*()
> > calls returning bdev_handle which is then
On Fri 25-08-23 02:58:43, Al Viro wrote:
> On Fri, Aug 11, 2023 at 01:04:31PM +0200, Jan Kara wrote:
> > Hello,
> >
> > this is a v2 of the patch series which implements the idea of
> > blkdev_get_by_*()
> > calls returning bdev_handle which is then passed to blk
Hello,
this is a v3 of the patch series which implements the idea of blkdev_get_by_*()
calls returning bdev_handle which is then passed to blkdev_put() [1]. This
makes the get and put calls for bdevs more obviously matching and allows us to
propagate context from get to put without having to modif
Convert xen/blkback to use bdev_open_by_dev() and pass the
handle around.
CC: xen-devel@lists.xenproject.org
Acked-by: Christoph Hellwig
Signed-off-by: Jan Kara
---
drivers/block/xen-blkback/blkback.c | 4 +--
drivers/block/xen-blkback/common.h | 4 +--
drivers/block/xen-blkback/xenbus.c
Hello,
this is a v2 of the patch series which implements the idea of blkdev_get_by_*()
calls returning bdev_handle which is then passed to blkdev_put() [1]. This
makes the get and put calls for bdevs more obviously matching and allows us to
propagate context from get to put without having to modif
Convert xen/blkback to use bdev_open_by_dev() and pass the
handle around.
CC: xen-devel@lists.xenproject.org
Signed-off-by: Jan Kara
---
drivers/block/xen-blkback/blkback.c | 4 +--
drivers/block/xen-blkback/common.h | 4 +--
drivers/block/xen-blkback/xenbus.c | 40
On Wed 12-07-23 18:06:35, Haris Iqbal wrote:
> On Thu, Jul 6, 2023 at 5:38 PM Christoph Hellwig wrote:
> >
> > On Tue, Jul 04, 2023 at 02:21:28PM +0200, Jan Kara wrote:
> > > Create struct bdev_handle that contains all parameters that need to be
> > > p
On Fri 07-07-23 04:28:41, Christoph Hellwig wrote:
> On Thu, Jul 06, 2023 at 06:14:33PM +0200, Jan Kara wrote:
> > > struct bdev_handle *bdev_open_by_path(dev_t dev, blk_mode_t mode,
> > > void *holder, const struct blk_holder_ops *hops);
> > > void b
On Tue 04-07-23 07:06:26, Bart Van Assche wrote:
> On 7/4/23 05:21, Jan Kara wrote:
> > +struct bdev_handle {
> > + struct block_device *bdev;
> > + void *holder;
> > +};
>
> Please explain in the patch description why a holder pointer is introduced
> in s
On Tue 04-07-23 10:28:36, Keith Busch wrote:
> On Tue, Jul 04, 2023 at 02:21:28PM +0200, Jan Kara wrote:
> > +struct bdev_handle *blkdev_get_handle_by_dev(dev_t dev, blk_mode_t mode,
> > + void *holder, const struct blk_holder_ops *hops)
> > +{
> > +
On Tue 04-07-23 13:43:51, Matthew Wilcox wrote:
> On Tue, Jul 04, 2023 at 02:21:28PM +0200, Jan Kara wrote:
> > +struct bdev_handle *blkdev_get_handle_by_dev(dev_t dev, blk_mode_t mode,
> > + void *holder, const struct blk_holder_ops *hops)
> > +{
> > +
C: Minchan Kim
CC: ocfs2-de...@oss.oracle.com
CC: reiserfs-de...@vger.kernel.org
CC: Sergey Senozhatsky
CC: Song Liu
CC: Sven Schnelle
CC: target-de...@vger.kernel.org
CC: Ted Tso
CC: Trond Myklebust
CC: xen-devel@lists.xenproject.org
Signed-off-by: Jan Kara
---
bl
eiserfs-de...@vger.kernel.org
CC: Sergey Senozhatsky
CC: Song Liu
CC: Sven Schnelle
CC: target-de...@vger.kernel.org
CC: Ted Tso
CC: Trond Myklebust
CC: xen-devel@lists.xenproject.org
Signed-off-by: Jan Kara
---
block/bdev.c | 47 ++
include/linu
Hello,
this patch series implements the idea of blkdev_get_by_*() calls returning
bdev_handle which is then passed to blkdev_put() [1]. This makes the get
and put calls for bdevs more obviously matching and allows us to propagate
context from get to put without having to modify all the users (agai
Convert xen/blkback to use blkdev_get_handle_by_dev() and pass the
handle around.
CC: xen-devel@lists.xenproject.org
Signed-off-by: Jan Kara
---
drivers/block/xen-blkback/blkback.c | 4 +--
drivers/block/xen-blkback/common.h | 4 +--
drivers/block/xen-blkback/xenbus.c | 40
On Tue 24-11-20 14:27:18, Christoph Hellwig wrote:
> sector_t is now always a u64, so this check is not needed.
>
> Signed-off-by: Christoph Hellwig
Looks good. You can add:
Reviewed-by: Jan Kara
Honza
> ---
>
On Tue 24-11-20 14:27:14, Christoph Hellwig wrote:
> set_blocksize is used by file systems to use their preferred buffer cache
> block size. Block drivers should not set it.
>
> Signed-off-by: Christoph Hellwig
Looks good to me. You can add:
Reviewed-
return -EBUSY;
> + }
> bdput(bdev);
> - return -EBUSY;
> }
>
> - zram->claim = true;
> - mutex_unlock(&bdev->bd_mutex);
> -
> + del_gendisk(zram->disk);
> zram_debugfs_unregister(zram);
> -
> - /* Make sure all the pending I/O are finished */
> - fsync_bdev(bdev);
> zram_reset_device(zram);
> - bdput(bdev);
>
> pr_info("Removed device: %s\n", zram->disk->disk_name);
>
> - del_gendisk(zram->disk);
> blk_cleanup_queue(zram->disk->queue);
> put_disk(zram->disk);
> kfree(zram);
> --
> 2.29.2
>
--
Jan Kara
SUSE Labs, CR
On Tue 24-11-20 14:27:11, Christoph Hellwig wrote:
> del_gendisk already calls fsync_bdev for every partition, no need
> to do this twice.
>
> Signed-off-by: Christoph Hellwig
Makes sense to me. You can add:
Reviewed-
gt; goto out;
>
> @@ -618,7 +611,7 @@ int thaw_bdev(struct block_device *bdev, struct
> super_block *sb)
> bdev->bd_fsfreeze_count++;
> out:
> mutex_unlock(&bdev->bd_fsfreeze_mutex);
> - return error;
> + return 0;
But we now won't return -EINVAL if this gets called e.g. with
bd_fsfreeze_count == 0, right?
Honza
--
Jan Kara
SUSE Labs, CR
On Tue 24-11-20 14:27:09, Christoph Hellwig wrote:
> Just open code the wait in the only caller of both functions.
>
> Signed-off-by: Christoph Hellwig
Fine by me. You can add:
Reviewed-by: Jan Kara
Honza
> ---
>
. You can add:
Reviewed-by: Jan Kara
Honza
> ---
> mm/filemap.c | 13 ++---
> 1 file changed, 6 insertions(+), 7 deletions(-)
>
> diff --git a/mm/filemap.c b/mm/filemap.c
> index d5e7c2029d16b4..4f583489
e really need new bdev for
this because from higher level POV this is completely new device. And the
old bdev needs to live on as long as it is open. So IMO we still need to
just unhash the inode and leave it lingering in the background.
Honza
--
Jan Kara
SUSE Labs, CR
On Fri 20-11-20 15:59:56, Matthew Wilcox wrote:
> On Fri, Nov 20, 2020 at 04:32:53PM +0100, Christoph Hellwig wrote:
> > On Fri, Nov 20, 2020 at 12:21:21PM +0100, Jan Kara wrote:
> > > > > AFAICT bd_size_lock is pointless after these changes so we can just
>
On Fri 20-11-20 10:08:20, Christoph Hellwig wrote:
> On Thu, Nov 19, 2020 at 01:05:25PM +0100, Jan Kara wrote:
> > > @@ -613,7 +613,7 @@ void guard_bio_eod(struct bio *bio)
> > > rcu_read_lock();
> > > part = __disk_get_part(bio->bi_disk,
On Fri 20-11-20 10:15:46, Christoph Hellwig wrote:
> On Thu, Nov 19, 2020 at 03:39:21PM +0100, Jan Kara wrote:
> > This patch is kind of difficult to review due to the size of mostly
> > mechanical changes mixed with not completely mechanical changes. Can we
> > perhaps spl
m[SB_FREEZE_LEVELS];
> };
>
> @@ -3132,8 +3132,6 @@ extern struct file_system_type *get_filesystem(struct
> file_system_type *fs);
> extern void put_filesystem(struct file_system_type *fs);
> extern struct file_system_type *get_fs_type(const char *name);
> extern struct super_block *get_super(struct block_device *);
> -extern struct super_block *get_super_thawed(struct block_device *);
> -extern struct super_block *get_super_exclusive_thawed(struct block_device
> *bdev);
> extern struct super_block *get_active_super(struct block_device *bdev);
> extern void drop_super(struct super_block *sb);
> extern void drop_super_exclusive(struct super_block *sb);
> --
> 2.29.2
>
--
Jan Kara
SUSE Labs, CR
ou can add:
Reviewed-by: Jan Kara
Honza
> ---
> mm/filemap.c | 9 -
> 1 file changed, 4 insertions(+), 5 deletions(-)
>
> diff --git a/mm/filemap.c b/mm/filemap.c
> index d5e7c2029d16b4..3e3531a757f
On Wed 18-11-20 09:47:56, Christoph Hellwig wrote:
> We can just dereference the point in struct gendisk instead. Also
> remove the now unused export.
>
> Signed-off-by: Christoph Hellwig
Looks good to me. You can add:
Reviewed-
_from_disk() or something like that?
>
> ssize_t part_size_show(struct device *dev, struct device_attribute *attr,
> char *buf);
Honza
--
Jan Kara
SUSE Labs, CR
(unsigned long long)part_nr_sects_read(part) >> 1,
> +bdev_nr_sectors(part->bdev) >> 1,
> disk_name(sgp, part->partno, buf));
> disk_part_iter_exit(&piter);
>
> @@ -1158,8 +1169,7 @@ ssize_t part_size_show(struct device *dev,
> {
> struct hd_struct *p = dev_to_part(dev);
>
> - return sprintf(buf, "%llu\n",
> - (unsigned long long)part_nr_sects_read(p));
> + return sprintf(buf, "%llu\n", bdev_nr_sectors(p->bdev));
Is sector_t really guaranteed to be unsigned long long?
Honza
--
Jan Kara
SUSE Labs, CR
-off-by: Christoph Hellwig
Just two nits below. Otherwise feel free to add:
Reviewed-by: Jan Kara
> @@ -1521,7 +1510,7 @@ static int __blkdev_get(struct block_device *bdev,
> fmode_t mode, void *holder,
> if (bdev->bd_bdi == &noop_backing_dev_info)
>
On Wed 18-11-20 09:47:52, Christoph Hellwig wrote:
> Stop passing the whole device as a separate argument given that it
> can be trivially deducted.
>
> Signed-off-by: Christoph Hellwig
The patch looks good to me. You can add:
Reviewed-
;part0);
> - if (hd_ref_init(&disk->part0))
> - goto out_free_part0;
> + if (hd_ref_init(&disk->part0)) {
> + hd_free_part(&disk->part0);
Aren't you missing kfree(disk) here?
> + return NULL;
> + }
>
> disk->minors = minors;
> rand_initialize_disk(disk);
Honza
--
Jan Kara
SUSE Labs, CR
On Thu 19-11-20 08:52:25, Christoph Hellwig wrote:
> On Wed, Nov 18, 2020 at 03:37:47PM +0100, Jan Kara wrote:
> > > -static inline dev_t blk_lookup_devt(const char *name, int partno)
> > > -{
> > > - dev_t devt = MKDEV(0, 0);
> > > - return devt;
On Wed 18-11-20 09:47:45, Christoph Hellwig wrote:
> Use put_device to put the device instead of poking into the internals
> and using kobject_put.
>
> Signed-off-by: Christoph Hellwig
Looks good. You can add:
Reviewed-
On Wed 18-11-20 09:47:48, Christoph Hellwig wrote:
> The code in devt_from_partuuid is very convoluted. Refactor a bit by
> sanitizing the goto and variable name usage.
>
> Signed-off-by: Christoph Hellwig
The patch looks good to me. You can add:
Reviewed-
On Wed 18-11-20 09:47:44, Christoph Hellwig wrote:
> Call disk_part_iter_exit in disk_part_iter_next instead of duplicating
> the functionality.
>
> Signed-off-by: Christoph Hellwig
OK. You can add:
Reviewed-by: Jan Kara
On Wed 18-11-20 09:47:49, Christoph Hellwig wrote:
> Avoid a totally pointless goto label, and use the same style of
> comparism for both helpers.
>
> Signed-off-by: Christoph Hellwig
OK. You can add:
Reviewed-by: Jan Kara
On Wed 18-11-20 09:47:43, Christoph Hellwig wrote:
> Add a little helper to find the kobject for a struct block_device.
>
> Signed-off-by: Christoph Hellwig
Looks good. You can add:
Reviewed-by: Jan Kara
Honza
> --
On Wed 18-11-20 09:47:50, Christoph Hellwig wrote:
> Reorder the code to have one big section for the last close, and to use
> bdev_is_partition.
>
> Signed-off-by: Christoph Hellwig
Looks good. You can add:
Reviewed-
On Wed 18-11-20 09:47:42, Christoph Hellwig wrote:
> Signed-off-by: Christoph Hellwig
Looks good. You can add:
Reviewed-by: Jan Kara
Honza
> ---
> include/linux/genhd.h | 1 -
> 1 file changed, 1 deletion(-)
>
On Wed 18-11-20 09:47:46, Christoph Hellwig wrote:
> Adding the minor to the major creates tons of pointless conflicts. Just
> use the dev_t itself, which is 32-bits and thus is guaranteed to fit
> into ino_t.
>
> Signed-off-by: Christoph Hellwig
Fair enough. You can add:
Review
e declaration
outside the CONFIG_BLOCK ifdef? AFAICS blk_lookup_devt() still exists only
when CONFIG_BLOCK is defined? Otherwise the patch looks good to me.
Honza
--
Jan Kara
SUSE Labs, CR
On Wed 18-11-20 09:47:41, Christoph Hellwig wrote:
> disk_get_part needs to be paired with a disk_put_part.
>
> Fixes: ef45fe470e1 ("blk-cgroup: show global disk stats in root cgroup
> io.stat")
> Signed-off-by: Christoph Hellwig
Looks good to me. You can ad
m now
> touching mm, procfs, rdma, ext4, and xfs.
MM tree would be one candidate for routing but there are other options that
would make sense as well - Dan's tree, VFS tree, or even I can pickup the
patches to my tree if needed. But let's worry about the routing after
ame I used while prototyping was
> pretty bad... So Thanks Jan... ;-)
For your function, I'd choose a name like vaddr_pin_leased_pages() so that
association with a lease is clear from the name :) Also I'd choose the
counterpart to be vaddr_unpin_leased_page[s](). Especially having put_page in
the name looks confusing to me...
Honza
--
Jan Kara
SUSE Labs, CR
___
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
On Fri 02-08-19 12:14:09, John Hubbard wrote:
> On 8/2/19 7:52 AM, Jan Kara wrote:
> > On Fri 02-08-19 07:24:43, Matthew Wilcox wrote:
> > > On Fri, Aug 02, 2019 at 02:41:46PM +0200, Jan Kara wrote:
> > > > On Fri 02-08-19 11:12:44, Michal Hocko wrote:
> > >
On Fri 02-08-19 07:24:43, Matthew Wilcox wrote:
> On Fri, Aug 02, 2019 at 02:41:46PM +0200, Jan Kara wrote:
> > On Fri 02-08-19 11:12:44, Michal Hocko wrote:
> > > On Thu 01-08-19 19:19:31, john.hubb...@gmail.com wrote:
> > > [...]
> > > > 2) Convert all of t
efs using
put_page()) but I suppose it would be a high enough barrier for missed
conversions... Thoughts?
Honza
--
Jan Kara
SUSE Labs, CR
___
Xen-devel mailing list
Xen-devel@lists.xen
65 matches
Mail list logo
