Re: [PATCH v7] sbitmap: fix io hung due to race on sbitmap_word::cleared

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On 7/15/24 2:11 AM, Yang Yang wrote:
+
+	/**
+	 * @swap_lock: serializes simultaneous updates of ->word and ->cleared
+	 */
+	spinlock_t swap_lock;
  } ____cacheline_aligned_in_smp;

Thank you for having updated this comment.

-static inline bool sbitmap_deferred_clear(struct sbitmap_word *map)
+static inline bool sbitmap_deferred_clear(struct sbitmap_word *map,
+		unsigned int depth, unsigned int alloc_hint, bool wrap)
  {
-	unsigned long mask;
+	unsigned long mask, word_mask;
+	bool ret = false;
- if (!READ_ONCE(map->cleared))
-		return false;
+	guard(spinlock_irqsave)(&map->swap_lock);
+
+	if (!map->cleared) {
+		if (depth > 0) {
+			word_mask = (~0UL) >> (BITS_PER_LONG - depth);
+			/*
+			 * The current behavior is to always retry after moving
+			 * ->cleared to word, and we change it to retry in case
+			 * of any free bits. To avoid an infinite loop, we need
+			 * to take wrap & alloc_hint into account, otherwise a
+			 * soft lockup may occur.
+			 */
+			if (!wrap && alloc_hint)
+				word_mask &= ~((1UL << alloc_hint) - 1);
+
+			if ((READ_ONCE(map->word) & word_mask) == word_mask)
+				ret = false;
+			else
+				ret = true;
+		}
+
+		return ret;
+	}

Now that guard()() is being used, the local variable 'ret' can be eliminated. The if (READ_ONCE() ...) statement can be changed into the
following: return (READ_ONCE(map->word) & word_mask) != word_mask;
and "return ret;" can be changed into "return false;". Additionally,
the indentation depth can be reduced by changing "if (depth > 0) {" ...
into "if (depth == 0) return false;".

Otherwise this patch looks good to me.

Thanks,

Bart.




[Index of Archives]     [Linux RAID]     [Linux SCSI]     [Linux ATA RAID]     [IDE]     [Linux Wireless]     [Linux Kernel]     [ATH6KL]     [Linux Bluetooth]     [Linux Netdev]     [Kernel Newbies]     [Security]     [Git]     [Netfilter]     [Bugtraq]     [Yosemite News]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Device Mapper]

  Powered by Linux