Re: [PATCH v5 1/5] fs, block: refactor enum rw_hint

Kanchan Joshi
2024-09-12T15:51:05Z

On 9/12/2024 6:23 PM, Christoph Hellwig wrote:
> On Tue, Sep 10, 2024 at 08:31:56PM +0530, Kanchan Joshi wrote:
>> Rename enum rw_hint to rw_lifetime_hint.
>> Change i_write_hint (in inode), bi_write_hint(in bio), and write_hint
>> (in request) to use u8 data-type rather than this enum.
>>
>> This is in preparation to introduce a new write hint type.
>
> The rationale seems a bit sparse. Why is it renamed? Because the
> name fits better, because you need the same for something else?
>

Right, new name fits better. Because 'enum rw_hint' is a generic name
that conveys 'any' hint. This was fine before. But once we start
supporting more than one hint type, we need to be specific what
hint-type is being handled. More below.

>> static void submit_bh_wbc(blk_opf_t opf, struct buffer_head *bh,
>> - enum rw_hint hint, struct writeback_control *wbc);
>> + u8 hint, struct writeback_control *wbc);
>
> And moving from the enum to an plain integer seems like a bit of a
> retrograde step.

This particular enum is hardwired to take 6 temperature-hint values [*].
But this (and many other) functions act as a simple propagator, which do
not have to care whether hint type is lifetime or placement or anything
else.

The creator/originator of the hint decides what hint to pass (userspace
in this case). And the consumer (driver in this case) decides whether or
not it understands the hint that has been passed. The intermediate
components/functions only need to pass the hint, regardless of its type,
down.

Wherever hint is being used in generic way, u8 data type is being used.
Down the line if a component/function needs to care for a specific
type, it can start decoding the passed hint type/value (using the
appropriate macro similar to what this series does for SCSI and NVMe).

Overall, this also helps to avoid the churn. Otherwise we duplicate all
the propagation code that has been done for temperature hint across the
IO stack.

[*]
enum rw_hint {
WRITE_LIFE_NOT_SET = RWH_WRITE_LIFE_NOT_SET,
WRITE_LIFE_NONE = RWH_WRITE_LIFE_NONE,
WRITE_LIFE_SHORT = RWH_WRITE_LIFE_SHORT,
WRITE_LIFE_MEDIUM = RWH_WRITE_LIFE_MEDIUM,
WRITE_LIFE_LONG = RWH_WRITE_LIFE_LONG,
WRITE_LIFE_EXTREME = RWH_WRITE_LIFE_EXTREME,
} __packed;