On 23/01/31 07:07AM, Christoph Hellwig wrote:
> On Mon, Jan 30, 2023 at 10:24:59PM +0000, Matthew Wilcox wrote:
> > > a single bitmap of undefined length, then change the declaration and
> > > the structure size calculation away from using array notation and
> > > instead just use pointers to the individual bitmap regions within
> > > the allocated region.
> >
> > Hard to stomach that solution when the bitmap is usually 2 bytes long
> > (in Ritesh's case). Let's see a version of this patchset with
> > accessors before rendering judgement.
>
> Yes. I think what we need is proper helpers that are self-documenting
> for every bitmap update as I already suggsted last round. That keeps
> the efficient allocation, and keeps all the users self-documenting.
> It just adds a bit of boilerplate for all these helpers, but that
> should be worth having the clarity and performance benefits.
Are these accessor apis looking good to be part of fs/iomap/buffered-io.c
The rest of the changes will then be just using this to set/clear/test the
uptodate/dirty bits of iop->state bitmap.
I think, after these apis, there shouldn't be any place where we need to
directly manipulate iop->state bitmap. These APIs are all what I think are
required for current changeset.
Please let me know your thoughts/suggestions on these.
If it looks good, then I can fold these in, in different patches which
implements uptodate/dirty functionality and send rfcv3 along with other
review comments addressed.
/*
* Accessor functions for setting/clearing/checking uptodate and
* dirty bits in iop->state bitmap.
* nrblocks is i_blocks_per_folio() which is passed in every
* function as the last argument for API consistency.
*/
static inline void iop_set_range_uptodate(struct iomap_page *iop,
unsigned int start, unsigned int len,
unsigned int nrblocks)
{
bitmap_set(iop->state, start, len);
}
static inline void iop_clear_range_uptodate(struct iomap_page *iop,
unsigned int start, unsigned int len,
unsigned int nrblocks)
{
bitmap_clear(iop->state, start, len);
}
static inline bool iop_test_uptodate(struct iomap_page *iop, unsigned int pos,
unsigned int nrblocks)
{
return test_bit(pos, iop->state);
}
static inline bool iop_full_uptodate(struct iomap_page *iop,
unsigned int nrblocks)
{
return bitmap_full(iop->state, nrblocks);
}
static inline void iop_set_range_dirty(struct iomap_page *iop,
unsigned int start, unsigned int len,
unsigned int nrblocks)
{
bitmap_set(iop->state, start + nrblocks, len);
}
static inline void iop_clear_range_dirty(struct iomap_page *iop,
unsigned int start, unsigned int len,
unsigned int nrblocks)
{
bitmap_clear(iop->state, start + nrblocks, len);
}
static inline bool iop_test_dirty(struct iomap_page *iop, unsigned int pos,
unsigned int nrblocks)
{
return test_bit(pos + nrblocks, iop->state);
}
-ritesh
