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

[Kanchan Joshi <joshi.k@samsung.com>]

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;