Skip to content
Draft
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
20 changes: 20 additions & 0 deletions arch/alpha/include/asm/bitops.h
Original file line number Diff line number Diff line change
Expand Up @@ -286,6 +286,26 @@ arch___test_and_change_bit(unsigned long nr, volatile unsigned long *addr)
#define arch_test_bit generic_test_bit
#define arch_test_bit_acquire generic_test_bit_acquire

static inline bool xor_unlock_is_negative_byte(unsigned long mask,
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

issue (complexity): Consider using GNU atomic builtins to replace the inline assembly loop with a single atomic XOR operation, if the platform supports it, to reduce complexity and duplicate logic while preserving functionality .

Consider using the GNU atomic builtins to simplify the inline assembly loop. For example, if the platform supports __atomic builtins, you can replace the inline assembly with a single atomic XOR operation:

static inline bool xor_unlock_is_negative_byte(unsigned long mask,
                                                 volatile unsigned long *p)
{
    unsigned long old = __atomic_fetch_xor(p, mask, __ATOMIC_ACQ_REL);
    return (old & BIT(7)) != 0;
}

This preserves the functionality while reducing the low-level complexity and duplicate logic.

volatile unsigned long *p)
{
unsigned long temp, old;

__asm__ __volatile__(
"1: ldl_l %0,%4\n"
" mov %0,%2\n"
" xor %0,%3,%0\n"
" stl_c %0,%1\n"
" beq %0,2f\n"
".subsection 2\n"
"2: br 1b\n"
".previous"
:"=&r" (temp), "=m" (*p), "=&r" (old)
:"Ir" (mask), "m" (*p));

return (old & BIT(7)) != 0;
}

/*
* ffz = Find First Zero in word. Undefined if no zero exists,
* so code should check against ~0UL first..
Expand Down
21 changes: 21 additions & 0 deletions arch/m68k/include/asm/bitops.h
Original file line number Diff line number Diff line change
Expand Up @@ -319,6 +319,27 @@ arch___test_and_change_bit(unsigned long nr, volatile unsigned long *addr)
return test_and_change_bit(nr, addr);
}

static inline bool xor_unlock_is_negative_byte(unsigned long mask,
volatile unsigned long *p)
{
#ifdef CONFIG_COLDFIRE
__asm__ __volatile__ ("eorl %1, %0"
: "+m" (*p)
: "d" (mask)
: "memory");
return *p & (1 << 7);
#else
char result;
char *cp = (char *)p + 3; /* m68k is big-endian */

__asm__ __volatile__ ("eor.b %1, %2; smi %0"
: "=d" (result)
: "di" (mask), "o" (*cp)
: "memory");
return result;
#endif
}

/*
* The true 68020 and more advanced processors support the "bfffo"
* instruction for finding bits. ColdFire and simple 68000 parts
Expand Down
25 changes: 24 additions & 1 deletion arch/mips/include/asm/bitops.h
Original file line number Diff line number Diff line change
Expand Up @@ -73,7 +73,8 @@ int __mips_test_and_clear_bit(unsigned long nr,
volatile unsigned long *addr);
int __mips_test_and_change_bit(unsigned long nr,
volatile unsigned long *addr);

bool __mips_xor_is_negative_byte(unsigned long mask,
volatile unsigned long *addr);

/*
* set_bit - Atomically set a bit in memory
Expand Down Expand Up @@ -279,6 +280,28 @@ static inline int test_and_change_bit(unsigned long nr,
return res;
}

static inline bool xor_unlock_is_negative_byte(unsigned long mask,
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

issue (complexity): Consider factoring out the conditional and inline assembly duplication into a helper function to improve code reuse and readability.

Consider factoring out the conditional and inline assembly duplication into a helper that picks the “mask” at a single point. For example:

```c
static inline unsigned long xor_unlock_atomic(volatile unsigned long *p,
                                              unsigned long effective_mask)
{
    unsigned long orig;
    if (!kernel_uses_llsc)
        orig = __mips_xor_is_negative_byte(effective_mask, p);
    else
        orig = __test_bit_op(*p, "%0", "xor\t%1, %0, %3", "ir"(effective_mask));
    return orig;
}

static inline bool xor_unlock_is_negative_byte(unsigned long mask,
                                                 volatile unsigned long *p)
{
    unsigned long eff_mask = kernel_uses_llsc ? BIT(7) : mask;
    unsigned long orig;

    smp_mb__before_atomic();
    orig = xor_unlock_atomic(p, eff_mask);
    smp_llsc_mb();

    return (orig & eff_mask) != 0;
}

This consolidates the conditional logic and reduces duplicate inline assembly while retaining original functionality.

volatile unsigned long *p)
{
unsigned long orig;
bool res;

smp_mb__before_atomic();

if (!kernel_uses_llsc) {
res = __mips_xor_is_negative_byte(mask, p);
} else {
orig = __test_bit_op(*p, "%0",
"xor\t%1, %0, %3",
"ir"(mask));
res = (orig & BIT(7)) != 0;
}

smp_llsc_mb();

return res;
}

#undef __bit_op
#undef __test_bit_op

Expand Down
14 changes: 14 additions & 0 deletions arch/mips/lib/bitops.c
Original file line number Diff line number Diff line change
Expand Up @@ -146,3 +146,17 @@ int __mips_test_and_change_bit(unsigned long nr, volatile unsigned long *addr)
return res;
}
EXPORT_SYMBOL(__mips_test_and_change_bit);

bool __mips_xor_is_negative_byte(unsigned long mask,
volatile unsigned long *addr)
{
unsigned long flags;
unsigned long data;

raw_local_irq_save(flags);
data = *addr;
*addr = data ^ mask;
raw_local_irq_restore(flags);

return (data & BIT(7)) != 0;
}
21 changes: 5 additions & 16 deletions arch/powerpc/include/asm/bitops.h
Original file line number Diff line number Diff line change
Expand Up @@ -233,35 +233,24 @@ static inline int arch_test_and_change_bit(unsigned long nr,
return test_and_change_bits(BIT_MASK(nr), addr + BIT_WORD(nr)) != 0;
}

#ifdef CONFIG_PPC64
static inline unsigned long
clear_bit_unlock_return_word(int nr, volatile unsigned long *addr)
static inline bool arch_xor_unlock_is_negative_byte(unsigned long mask,
volatile unsigned long *p)
{
unsigned long old, t;
unsigned long *p = (unsigned long *)addr + BIT_WORD(nr);
unsigned long mask = BIT_MASK(nr);

__asm__ __volatile__ (
PPC_RELEASE_BARRIER
"1:" PPC_LLARX "%0,0,%3,0\n"
"andc %1,%0,%2\n"
"xor %1,%0,%2\n"
PPC_STLCX "%1,0,%3\n"
"bne- 1b\n"
: "=&r" (old), "=&r" (t)
: "r" (mask), "r" (p)
: "cc", "memory");

return old;
return (old & BIT_MASK(7)) != 0;
}

/*
* This is a special function for mm/filemap.c
* Bit 7 corresponds to PG_waiters.
*/
#define arch_clear_bit_unlock_is_negative_byte(nr, addr) \
(clear_bit_unlock_return_word(nr, addr) & BIT_MASK(7))

#endif /* CONFIG_PPC64 */
#define arch_xor_unlock_is_negative_byte arch_xor_unlock_is_negative_byte

#include <asm-generic/bitops/non-atomic.h>

Expand Down
12 changes: 12 additions & 0 deletions arch/riscv/include/asm/bitops.h
Original file line number Diff line number Diff line change
Expand Up @@ -191,6 +191,18 @@ static inline void __clear_bit_unlock(
clear_bit_unlock(nr, addr);
}

static inline bool xor_unlock_is_negative_byte(unsigned long mask,
volatile unsigned long *addr)
{
unsigned long res;
__asm__ __volatile__ (
__AMO(xor) ".rl %0, %2, %1"
: "=r" (res), "+A" (*addr)
: "r" (__NOP(mask))
: "memory");
return (res & BIT(7)) != 0;
}

#undef __test_and_op_bit
#undef __op_bit
#undef __NOP
Expand Down
10 changes: 10 additions & 0 deletions arch/s390/include/asm/bitops.h
Original file line number Diff line number Diff line change
Expand Up @@ -201,6 +201,16 @@ static inline void arch___clear_bit_unlock(unsigned long nr,
arch___clear_bit(nr, ptr);
}

static inline bool arch_xor_unlock_is_negative_byte(unsigned long mask,
volatile unsigned long *ptr)
{
unsigned long old;

old = __atomic64_xor_barrier(mask, (long *)ptr);
return old & BIT(7);
}
#define arch_xor_unlock_is_negative_byte arch_xor_unlock_is_negative_byte

#include <asm-generic/bitops/instrumented-atomic.h>
#include <asm-generic/bitops/instrumented-non-atomic.h>
#include <asm-generic/bitops/instrumented-lock.h>
Expand Down
11 changes: 5 additions & 6 deletions arch/x86/include/asm/bitops.h
Original file line number Diff line number Diff line change
Expand Up @@ -94,18 +94,17 @@ arch___clear_bit(unsigned long nr, volatile unsigned long *addr)
asm volatile(__ASM_SIZE(btr) " %1,%0" : : ADDR, "Ir" (nr) : "memory");
}

static __always_inline bool
arch_clear_bit_unlock_is_negative_byte(long nr, volatile unsigned long *addr)
static __always_inline bool arch_xor_unlock_is_negative_byte(unsigned long mask,
volatile unsigned long *addr)
{
bool negative;
asm volatile(LOCK_PREFIX "andb %2,%1"
asm volatile(LOCK_PREFIX "xorb %2,%1"
CC_SET(s)
: CC_OUT(s) (negative), WBYTE_ADDR(addr)
: "ir" ((char) ~(1 << nr)) : "memory");
: "iq" ((char)mask) : "memory");
return negative;
}
#define arch_clear_bit_unlock_is_negative_byte \
arch_clear_bit_unlock_is_negative_byte
#define arch_xor_unlock_is_negative_byte arch_xor_unlock_is_negative_byte

static __always_inline void
arch___clear_bit_unlock(long nr, volatile unsigned long *addr)
Expand Down
16 changes: 4 additions & 12 deletions fs/buffer.c
Original file line number Diff line number Diff line change
Expand Up @@ -282,13 +282,7 @@ static void end_buffer_async_read(struct buffer_head *bh, int uptodate)
} while (tmp != bh);
spin_unlock_irqrestore(&first->b_uptodate_lock, flags);

/*
* If all of the buffers are uptodate then we can set the page
* uptodate.
*/
if (folio_uptodate)
folio_mark_uptodate(folio);
folio_unlock(folio);
folio_end_read(folio, folio_uptodate);
return;

still_busy:
Expand Down Expand Up @@ -2439,12 +2433,10 @@ int block_read_full_folio(struct folio *folio, get_block_t *get_block)

if (!nr) {
/*
* All buffers are uptodate - we can set the folio uptodate
* as well. But not if get_block() returned an error.
* All buffers are uptodate or get_block() returned an
* error when trying to map them - we can finish the read.
*/
if (!page_error)
folio_mark_uptodate(folio);
folio_unlock(folio);
folio_end_read(folio, !page_error);
return 0;
}

Expand Down
14 changes: 3 additions & 11 deletions fs/ext4/readpage.c
Original file line number Diff line number Diff line change
Expand Up @@ -70,15 +70,8 @@ static void __read_end_io(struct bio *bio)
{
struct folio_iter fi;

bio_for_each_folio_all(fi, bio) {
struct folio *folio = fi.folio;

if (bio->bi_status)
folio_clear_uptodate(folio);
else
folio_mark_uptodate(folio);
folio_unlock(folio);
}
bio_for_each_folio_all(fi, bio)
folio_end_read(fi.folio, bio->bi_status == 0);
if (bio->bi_private)
mempool_free(bio->bi_private, bio_post_read_ctx_pool);
bio_put(bio);
Expand Down Expand Up @@ -335,8 +328,7 @@ int ext4_mpage_readpages(struct inode *inode,
if (ext4_need_verity(inode, folio->index) &&
!fsverity_verify_folio(folio))
goto set_error_page;
folio_mark_uptodate(folio);
folio_unlock(folio);
folio_end_read(folio, true);
continue;
}
} else if (fully_mapped) {
Expand Down
57 changes: 35 additions & 22 deletions fs/iomap/buffered-io.c
Original file line number Diff line number Diff line change
Expand Up @@ -29,9 +29,9 @@ typedef int (*iomap_punch_t)(struct inode *inode, loff_t offset, loff_t length);
* and I/O completions.
*/
struct iomap_folio_state {
atomic_t read_bytes_pending;
atomic_t write_bytes_pending;
spinlock_t state_lock;
unsigned int read_bytes_pending;
atomic_t write_bytes_pending;

/*
* Each block has two bits in this bitmap:
Expand All @@ -57,30 +57,32 @@ static inline bool ifs_block_is_uptodate(struct iomap_folio_state *ifs,
return test_bit(block, ifs->state);
}

static void ifs_set_range_uptodate(struct folio *folio,
static bool ifs_set_range_uptodate(struct folio *folio,
struct iomap_folio_state *ifs, size_t off, size_t len)
{
struct inode *inode = folio->mapping->host;
unsigned int first_blk = off >> inode->i_blkbits;
unsigned int last_blk = (off + len - 1) >> inode->i_blkbits;
unsigned int nr_blks = last_blk - first_blk + 1;
unsigned long flags;

spin_lock_irqsave(&ifs->state_lock, flags);
bitmap_set(ifs->state, first_blk, nr_blks);
if (ifs_is_fully_uptodate(folio, ifs))
folio_mark_uptodate(folio);
spin_unlock_irqrestore(&ifs->state_lock, flags);
return ifs_is_fully_uptodate(folio, ifs);
}

static void iomap_set_range_uptodate(struct folio *folio, size_t off,
size_t len)
{
struct iomap_folio_state *ifs = folio->private;
unsigned long flags;
bool uptodate = true;

if (ifs)
ifs_set_range_uptodate(folio, ifs, off, len);
else
if (ifs) {
spin_lock_irqsave(&ifs->state_lock, flags);
uptodate = ifs_set_range_uptodate(folio, ifs, off, len);
spin_unlock_irqrestore(&ifs->state_lock, flags);
}

if (uptodate)
folio_mark_uptodate(folio);
}

Expand Down Expand Up @@ -181,7 +183,7 @@ static void ifs_free(struct folio *folio)

if (!ifs)
return;
WARN_ON_ONCE(atomic_read(&ifs->read_bytes_pending));
WARN_ON_ONCE(ifs->read_bytes_pending != 0);
WARN_ON_ONCE(atomic_read(&ifs->write_bytes_pending));
WARN_ON_ONCE(ifs_is_fully_uptodate(folio, ifs) !=
folio_test_uptodate(folio));
Expand Down Expand Up @@ -249,20 +251,28 @@ static void iomap_adjust_read_range(struct inode *inode, struct folio *folio,
*lenp = plen;
}

static void iomap_finish_folio_read(struct folio *folio, size_t offset,
static void iomap_finish_folio_read(struct folio *folio, size_t off,
size_t len, int error)
{
struct iomap_folio_state *ifs = folio->private;
bool uptodate = !error;
bool finished = true;

if (unlikely(error)) {
folio_clear_uptodate(folio);
folio_set_error(folio);
} else {
iomap_set_range_uptodate(folio, offset, len);
if (ifs) {
unsigned long flags;

spin_lock_irqsave(&ifs->state_lock, flags);
if (!error)
uptodate = ifs_set_range_uptodate(folio, ifs, off, len);
ifs->read_bytes_pending -= len;
finished = !ifs->read_bytes_pending;
spin_unlock_irqrestore(&ifs->state_lock, flags);
}

if (!ifs || atomic_sub_and_test(len, &ifs->read_bytes_pending))
folio_unlock(folio);
if (error)
folio_set_error(folio);
if (finished)
folio_end_read(folio, uptodate);
}

static void iomap_read_end_io(struct bio *bio)
Expand Down Expand Up @@ -359,8 +369,11 @@ static loff_t iomap_readpage_iter(const struct iomap_iter *iter,
}

ctx->cur_folio_in_bio = true;
if (ifs)
atomic_add(plen, &ifs->read_bytes_pending);
if (ifs) {
spin_lock_irq(&ifs->state_lock);
ifs->read_bytes_pending += plen;
spin_unlock_irq(&ifs->state_lock);
}

sector = iomap_sector(iomap, pos);
if (!ctx->bio ||
Expand Down
Loading
Loading