On Sat, 3 Aug 2024 at 17:34, Al Viro <viro@xxxxxxxxxxxxxxxxxx> wrote:
>
> Bitmap handling in there certainly needs to be cleaned up; no arguments
> here. If anything, we might want something like
>
> bitmap_copy_and_extend(unsigned long *to, const unsigned long *from,
> unsigned int count, unsigned int size)
> {
> unsigned int copy = BITS_TO_LONGS(count);
>
> memcpy(to, from, copy * sizeof(long));
> if (count % BITS_PER_LONG)
> to[copy - 1] &= BITMAP_LAST_WORD_MASK(count);
> memset(to + copy, 0, bitmap_size(size) - copy * sizeof(long));
> }
>
> and use it for all of three bitmaps.
That certainly works for me.
If you want to specialize it a bit more, you might do a separate
routine for the "known to be word aligned" case.
I extended on my previous patch. I'm not sure it's worth it, and it's
still ENTIRELY UNTESTED, but you get the idea..
But I'm ok with your generic bitmap version too.
Linus
fs/file.c | 52 +++++++++++++++++++++++++++++++++++++++++-----------
1 file changed, 41 insertions(+), 11 deletions(-)
diff --git a/fs/file.c b/fs/file.c
index a11e59b5d602..948856d8caee 100644
--- a/fs/file.c
+++ b/fs/file.c
@@ -46,27 +46,57 @@ static void free_fdtable_rcu(struct rcu_head *rcu)
#define BITBIT_NR(nr) BITS_TO_LONGS(BITS_TO_LONGS(nr))
#define BITBIT_SIZE(nr) (BITBIT_NR(nr) * sizeof(long))
+#define COPY_WORDS(dst,src,n) do { *dst++ = *src++; } while (--n)
+#define CLEAR_WORDS(dst,n) do { *dst++ = 0; } while (--n)
+
+static void copy_bitmap_words(unsigned long *dst, unsigned long *src,
+ unsigned int copy, unsigned int clear)
+{
+ /* There is always at least one word in the fd bitmap! */
+ COPY_WORDS(dst, src, copy);
+ /* .. but might not be anything to clear */
+ if (clear) CLEAR_WORDS(dst, clear);
+}
+
+static void copy_bitmap_bits(unsigned long *dst, unsigned long *src,
+ unsigned int copy, unsigned int clear)
+{
+ if (copy >= BITS_PER_LONG) {
+ unsigned int words = copy / BITS_PER_LONG;
+ COPY_WORDS(dst, src, words);
+ copy &= BITS_PER_LONG-1;
+ }
+ if (copy) {
+ *dst = *src & ((1ul << copy)-1);
+ dst++;
+ }
+ /* The above will have dealt with the low bits of 'clear' too */
+ clear /= BITS_PER_LONG;
+ if (clear) CLEAR_WORDS(dst, clear);
+}
+
/*
* Copy 'count' fd bits from the old table to the new table and clear the extra
* space if any. This does not copy the file pointers. Called with the files
* spinlock held for write.
+ *
+ * Note: the fd bitmaps are always full bitmap words (see sane_fdtable_size()),
+ * but the full_fds_bits bitmap is a bitmap of fd bitmap words, not of fds.
*/
static void copy_fd_bitmaps(struct fdtable *nfdt, struct fdtable *ofdt,
unsigned int count)
{
- unsigned int cpy, set;
+ unsigned int copy, clear;
- cpy = count / BITS_PER_BYTE;
- set = (nfdt->max_fds - count) / BITS_PER_BYTE;
- memcpy(nfdt->open_fds, ofdt->open_fds, cpy);
- memset((char *)nfdt->open_fds + cpy, 0, set);
- memcpy(nfdt->close_on_exec, ofdt->close_on_exec, cpy);
- memset((char *)nfdt->close_on_exec + cpy, 0, set);
+ copy = count / BITS_PER_LONG;
+ clear = nfdt->max_fds / BITS_PER_LONG - copy;
- cpy = BITBIT_SIZE(count);
- set = BITBIT_SIZE(nfdt->max_fds) - cpy;
- memcpy(nfdt->full_fds_bits, ofdt->full_fds_bits, cpy);
- memset((char *)nfdt->full_fds_bits + cpy, 0, set);
+ /* Copy the bitmap words */
+ copy_bitmap_words(nfdt->open_fds, ofdt->open_fds, copy, clear);
+ copy_bitmap_words(nfdt->close_on_exec, ofdt->close_on_exec, copy, clear);
+
+ /* Copy the bits for the words */
+ copy_bitmap_bits(nfdt->full_fds_bits, ofdt->full_fds_bits, copy, clear);
}
/*
