Re: [PATCH 4/8] block: add a store_limit operations for sysfs entries

Christoph Hellwig
2025-01-07T08:23:10Z

Hi Ming,

this is a friendly reminder to reply without quoting the entire mail.
I did not find any content after scrolling half a dozend page of
full quote and gave up.

> > + .attr = { .name = _name, .mode = 0644 }, \
> > + .show = _prefix##_show, \
> > + .store_limit = _prefix##_store, \
> > +}
> > +
> > #define QUEUE_RW_LOAD_MODULE_ENTRY(_prefix, _name) \
> > static struct queue_sysfs_entry _prefix##_entry = { \
> > .attr = { .name = _name, .mode = 0644 }, \
> > @@ -441,7 +422,7 @@ static struct queue_sysfs_entry _prefix##_entry = { \
> >
> > QUEUE_RW_ENTRY(queue_requests, "nr_requests");
> > QUEUE_RW_ENTRY(queue_ra, "read_ahead_kb");
> > -QUEUE_RW_ENTRY(queue_max_sectors, "max_sectors_kb");
> > +QUEUE_LIM_RW_ENTRY(queue_max_sectors, "max_sectors_kb");
> > QUEUE_RO_ENTRY(queue_max_hw_sectors, "max_hw_sectors_kb");
> > QUEUE_RO_ENTRY(queue_max_segments, "max_segments");
> > QUEUE_RO_ENTRY(queue_max_integrity_segments, "max_integrity_segments");
> > @@ -457,7 +438,7 @@ QUEUE_RO_ENTRY(queue_io_opt, "optimal_io_size");
> > QUEUE_RO_ENTRY(queue_max_discard_segments, "max_discard_segments");
> > QUEUE_RO_ENTRY(queue_discard_granularity, "discard_granularity");
> > QUEUE_RO_ENTRY(queue_max_hw_discard_sectors, "discard_max_hw_bytes");
> > -QUEUE_RW_ENTRY(queue_max_discard_sectors, "discard_max_bytes");
> > +QUEUE_LIM_RW_ENTRY(queue_max_discard_sectors, "discard_max_bytes");
> > QUEUE_RO_ENTRY(queue_discard_zeroes_data, "discard_zeroes_data");
> >
> > QUEUE_RO_ENTRY(queue_atomic_write_max_sectors, "atomic_write_max_bytes");
> > @@ -477,11 +458,11 @@ QUEUE_RO_ENTRY(queue_max_open_zones, "max_open_zones");
> > QUEUE_RO_ENTRY(queue_max_active_zones, "max_active_zones");
> >
> > QUEUE_RW_ENTRY(queue_nomerges, "nomerges");
> > -QUEUE_RW_ENTRY(queue_iostats_passthrough, "iostats_passthrough");
> > +QUEUE_LIM_RW_ENTRY(queue_iostats_passthrough, "iostats_passthrough");
> > QUEUE_RW_ENTRY(queue_rq_affinity, "rq_affinity");
> > QUEUE_RW_ENTRY(queue_poll, "io_poll");
> > QUEUE_RW_ENTRY(queue_poll_delay, "io_poll_delay");
> > -QUEUE_RW_ENTRY(queue_wc, "write_cache");
> > +QUEUE_LIM_RW_ENTRY(queue_wc, "write_cache");
> > QUEUE_RO_ENTRY(queue_fua, "fua");
> > QUEUE_RO_ENTRY(queue_dax, "dax");
> > QUEUE_RW_ENTRY(queue_io_timeout, "io_timeout");
> > @@ -494,10 +475,10 @@ static struct queue_sysfs_entry queue_hw_sector_size_entry = {
> > .show = queue_logical_block_size_show,
> > };
> >
> > -QUEUE_RW_ENTRY(queue_rotational, "rotational");
> > -QUEUE_RW_ENTRY(queue_iostats, "iostats");
> > -QUEUE_RW_ENTRY(queue_add_random, "add_random");
> > -QUEUE_RW_ENTRY(queue_stable_writes, "stable_writes");
> > +QUEUE_LIM_RW_ENTRY(queue_rotational, "rotational");
> > +QUEUE_LIM_RW_ENTRY(queue_iostats, "iostats");
> > +QUEUE_LIM_RW_ENTRY(queue_add_random, "add_random");
> > +QUEUE_LIM_RW_ENTRY(queue_stable_writes, "stable_writes");
> >
> > #ifdef CONFIG_BLK_WBT
> > static ssize_t queue_var_store64(s64 *var, const char *page)
> > @@ -695,7 +676,7 @@ queue_attr_store(struct kobject *kobj, struct attribute *attr,
> > struct request_queue *q = disk->queue;
> > ssize_t res;
> >
> > - if (!entry->store)
> > + if (!entry->store_limit && !entry->store)
> > return -EIO;
> >
> > /*
> > @@ -706,11 +687,24 @@ queue_attr_store(struct kobject *kobj, struct attribute *attr,
> > if (entry->load_module)
> > entry->load_module(disk, page, length);
> >
> > - blk_mq_freeze_queue(q);
> > mutex_lock(&q->sysfs_lock);
> > - res = entry->store(disk, page, length);
> > - mutex_unlock(&q->sysfs_lock);
> > + blk_mq_freeze_queue(q);
>
> Order between freeze and ->sysfs_lock is changed, and it may cause new
> lockdep warning because we may freeze queue first before acquiring
> ->sysfs_lock in del_gendisk().
>
>
> thanks,
> Ming
---end quoted text---