linux-fsdevel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Christoph Hellwig <hch@lst.de>
To: Kanchan Joshi <joshi.k@samsung.com>
Cc: Christoph Hellwig <hch@lst.de>,
	axboe@kernel.dk, kbusch@kernel.org, sagi@grimberg.me,
	martin.petersen@oracle.com,
	James.Bottomley@HansenPartnership.com, brauner@kernel.org,
	viro@zeniv.linux.org.uk, jack@suse.cz, jaegeuk@kernel.org,
	jlayton@kernel.org, chuck.lever@oracle.com, bvanassche@acm.org,
	linux-nvme@lists.infradead.org, linux-fsdevel@vger.kernel.org,
	linux-f2fs-devel@lists.sourceforge.net,
	linux-block@vger.kernel.org, linux-scsi@vger.kernel.org,
	gost.dev@samsung.com, vishak.g@samsung.com,
	javier.gonz@samsung.com, Nitesh Shetty <nj.shetty@samsung.com>
Subject: Re: [PATCH v5 4/5] sd: limit to use write life hints
Date: Fri, 13 Sep 2024 10:06:59 +0200	[thread overview]
Message-ID: <20240913080659.GA30525@lst.de> (raw)
In-Reply-To: <e6ae5391-ae84-bae4-78ea-4983d04af69f@samsung.com>

On Thu, Sep 12, 2024 at 10:01:00PM +0530, Kanchan Joshi wrote:
> Please see the response in patch #1. My worries were:
> (a) adding a new field and propagating it across the stack will cause 
> code duplication.
> (b) to add a new field we need to carve space within inode, bio and 
> request.
> We had a hole in request, but it is set to vanish after ongoing 
> integrity refactoring patch of Keith [1]. For inode also, there is no 
> liberty at this point [2].
> 
> I think current multiplexing approach is similar to ioprio where 
> multiple io priority classes/values are expressed within an int type. 
> And few kernel components choose to interpret certain ioprio values at will.
> 
> And all this is still in-kernel details. Which can be changed if/when 
> other factors start helping.

Maybe part of the problem is that the API is very confusing.  A smal
part of that is of course that the existing temperature hints already
have some issues, but this seems to be taking them make it significantly
worse.

Note: this tries to include highlevel comments from the discussion of
the previous patches instead of splitting them over multiple threads.

F_{S,G}ET_RW_HINT works on arbitrary file descriptors with absolutely no
check for support by the device or file system and not check for the
file type.  That's not exactly good API design, but not really a major
because they are clearly designed as hints with a fixed number of
values, allowing the implementation to map them if not enough are
supported.

But if we increase this to a variable number of hints that don't have
any meaning (and even if that is just the rough order of the temperature
hints assigned to them), that doesn't really work.  We'll need an API
to check if these stream hints are supported and how many of them,
otherwise the applications can't make any sensible use of them.

If these aren't just stream hints of the file system but you actually
want them as an abstract API for FDP you'll also need to actually
expose even more information like the reclaim unit size, but let's
ignore that for this part of the discssion.

Back the the API: the existing lifetime hints have basically three
layers:

 1) syscall ABI
 2) the hint stored in the inode
 3) the hint passed in the bio

1) is very much fixed for the temperature API, we just need to think if
   we want to support it at the same time as a more general hints API.
   Or if we can map one into another.  Or if we can't support them at
   the same time how that is communicated.

For 2) and 3) we can use an actual union if we decide to not support
both at the same time, keyed off a flag outside the field, but if not
we simply need space for both.

  reply	other threads:[~2024-09-13  8:07 UTC|newest]

Thread overview: 31+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
     [not found] <CGME20240910151040epcas5p3f47fa7ea37a35f8b44dd9174689e1bb9@epcas5p3.samsung.com>
2024-09-10 15:01 ` [PATCH v5 0/5] data placement hints and FDP Kanchan Joshi
     [not found]   ` <CGME20240910151044epcas5p37f61bb85ccf8b3eb875e77c3fc260c51@epcas5p3.samsung.com>
2024-09-10 15:01     ` [PATCH v5 1/5] fs, block: refactor enum rw_hint Kanchan Joshi
2024-09-12 12:53       ` Christoph Hellwig
2024-09-12 15:50         ` Kanchan Joshi
2024-09-12 20:30           ` Bart Van Assche
2024-09-13  7:22             ` Kanchan Joshi
     [not found]   ` <CGME20240910151048epcas5p3c610d63022362ec5fcc6fc362ad2fb9f@epcas5p3.samsung.com>
2024-09-10 15:01     ` [PATCH v5 2/5] fcntl: rename rw_hint_* to rw_lifetime_hint_* Kanchan Joshi
2024-09-12 12:54       ` Christoph Hellwig
2024-09-12 15:51         ` Kanchan Joshi
     [not found]   ` <CGME20240910151052epcas5p48b20962753b1e3171daf98f050d0b5af@epcas5p4.samsung.com>
2024-09-10 15:01     ` [PATCH v5 3/5] fcntl: add F_{SET/GET}_RW_HINT_EX Kanchan Joshi
2024-09-10 18:48       ` Jens Axboe
2024-09-11 15:50         ` Kanchan Joshi
2024-09-12 13:01       ` Christoph Hellwig
2024-09-12 15:53         ` Kanchan Joshi
2024-09-12 20:36       ` Bart Van Assche
2024-09-13  7:15         ` Kanchan Joshi
     [not found]   ` <CGME20240910151057epcas5p3369c6257a6f169b4caa6dd59548b538c@epcas5p3.samsung.com>
2024-09-10 15:01     ` [PATCH v5 4/5] sd: limit to use write life hints Kanchan Joshi
2024-09-12 13:02       ` Christoph Hellwig
2024-09-12 16:31         ` Kanchan Joshi
2024-09-13  8:06           ` Christoph Hellwig [this message]
2024-09-16 13:49             ` Kanchan Joshi
2024-09-17  6:20               ` Christoph Hellwig
2024-09-17 16:03                 ` Kanchan Joshi
2024-09-17 17:00                   ` Kanchan Joshi
2024-09-18  6:42                   ` Christoph Hellwig
2024-09-18  8:12                     ` Kanchan Joshi
2024-09-18 12:01                       ` Christoph Hellwig
2024-09-24  9:24                         ` Kanchan Joshi
2024-09-24  9:28                           ` Christoph Hellwig
     [not found]   ` <CGME20240910151101epcas5p1c4e90f7334125fc49106d58d43cffcec@epcas5p1.samsung.com>
2024-09-10 15:02     ` [PATCH v5 5/5] nvme: enable FDP support Kanchan Joshi
2025-01-29  0:56   ` [f2fs-dev] [PATCH v5 0/5] data placement hints and FDP patchwork-bot+f2fs

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20240913080659.GA30525@lst.de \
    --to=hch@lst.de \
    --cc=James.Bottomley@HansenPartnership.com \
    --cc=axboe@kernel.dk \
    --cc=brauner@kernel.org \
    --cc=bvanassche@acm.org \
    --cc=chuck.lever@oracle.com \
    --cc=gost.dev@samsung.com \
    --cc=jack@suse.cz \
    --cc=jaegeuk@kernel.org \
    --cc=javier.gonz@samsung.com \
    --cc=jlayton@kernel.org \
    --cc=joshi.k@samsung.com \
    --cc=kbusch@kernel.org \
    --cc=linux-block@vger.kernel.org \
    --cc=linux-f2fs-devel@lists.sourceforge.net \
    --cc=linux-fsdevel@vger.kernel.org \
    --cc=linux-nvme@lists.infradead.org \
    --cc=linux-scsi@vger.kernel.org \
    --cc=martin.petersen@oracle.com \
    --cc=nj.shetty@samsung.com \
    --cc=sagi@grimberg.me \
    --cc=viro@zeniv.linux.org.uk \
    --cc=vishak.g@samsung.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).