Re: [PATCH 1/4] bdi: Track users that require stable page writes

[Christoph Hellwig]

> +static inline void bdi_require_stable_pages(struct backing_dev_info *bdi)
> +{
> +	bdi->capabilities |= BDI_CAP_STABLE_WRITES;
> +}
> +
> +static inline void bdi_unrequire_stable_pages(struct backing_dev_info *bdi)
> +{
> +	bdi->capabilities &= ~BDI_CAP_STABLE_WRITES;
> +}

Any reason to provide these wrappers while other BDI_CAP_ values don't
have it/

Also what protects bdi->capabilities against concurrent updates now that
it gets modified at runtime?

> +static inline void queue_require_stable_pages(struct request_queue *q)
> +{
> +	bdi_require_stable_pages(&q->backing_dev_info);
> +}
> +
> +static inline void queue_unrequire_stable_pages(struct request_queue *q)
> +{
> +	bdi_unrequire_stable_pages(&q->backing_dev_info);
> +}
> +
> +static inline int queue_requires_stable_pages(struct request_queue *q)
> +{
> +	return bdi_cap_stable_pages_required(&q->backing_dev_info);
> +}

Independent of the above I see no point in these wrappers that just
provide a single dereference.

> +static ssize_t stable_pages_required_store(struct device *dev,
> +					   struct device_attribute *attr,
> +					   const char *buf, size_t count)

Can you add a rationale on why we'd want to allow users to change the
value?  I can't really think of any.

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/