[bug report] netfs: Speed up buffered reading

[Dan Carpenter <dan.carpenter@linaro.org>]

Hello David Howells,

Commit 5c7822a3013d ("netfs: Speed up buffered reading") from Jul 2,
2024 (linux-next), leads to the following Smatch static checker
warning:

	fs/netfs/read_collect.c:175 netfs_consume_read_data()
	error: we previously assumed 'folioq' could be null (see line 115)

fs/netfs/read_collect.c
    86 static bool netfs_consume_read_data(struct netfs_io_subrequest *subreq, bool was_async)
    87 {
    88         struct netfs_io_subrequest *prev, *next;
    89         struct netfs_io_request *rreq = subreq->rreq;
    90         struct folio_queue *folioq = subreq->curr_folioq;
    91         size_t avail, prev_donated, next_donated, fsize, part, excess;
    92         loff_t fpos, start;
    93         loff_t fend;
    94         int slot = subreq->curr_folioq_slot;
    95 
    96         if (WARN(subreq->transferred > subreq->len,
    97                  "Subreq overread: R%x[%x] %zu > %zu",
    98                  rreq->debug_id, subreq->debug_index,
    99                  subreq->transferred, subreq->len))
    100                 subreq->transferred = subreq->len;
    101 
    102 next_folio:
    103         fsize = PAGE_SIZE << subreq->curr_folio_order;
    104         fpos = round_down(subreq->start + subreq->consumed, fsize);
    105         fend = fpos + fsize;
    106 
    107         if (WARN_ON_ONCE(!folioq) ||
                                 ^^^^^^^
Check for NULL

    108             WARN_ON_ONCE(!folioq_folio(folioq, slot)) ||
    109             WARN_ON_ONCE(folioq_folio(folioq, slot)->index != fpos / PAGE_SIZE)) {
    110                 pr_err("R=%08x[%x] s=%llx-%llx ctl=%zx/%zx/%zx sl=%u\n",
    111                        rreq->debug_id, subreq->debug_index,
    112                        subreq->start, subreq->start + subreq->transferred - 1,
    113                        subreq->consumed, subreq->transferred, subreq->len,
    114                        slot);
    115                 if (folioq) {
    116                         struct folio *folio = folioq_folio(folioq, slot);
    117 
    118                         pr_err("folioq: orders=%02x%02x%02x%02x\n",
    119                                folioq->orders[0], folioq->orders[1],
    120                                folioq->orders[2], folioq->orders[3]);
    121                         if (folio)
    122                                 pr_err("folio: %llx-%llx ix=%llx o=%u qo=%u\n",
    123                                        fpos, fend - 1, folio_pos(folio), folio_order(folio),
    124                                        folioq_folio_order(folioq, slot));
    125                 }
    126         }
    127 
    128 donation_changed:
    129         /* Try to consume the current folio if we've hit or passed the end of
    130          * it.  There's a possibility that this subreq doesn't start at the
    131          * beginning of the folio, in which case we need to donate to/from the
    132          * preceding subreq.
    133          *
    134          * We also need to include any potential donation back from the
    135          * following subreq.
    136          */
    137         prev_donated = READ_ONCE(subreq->prev_donated);
    138         next_donated =  READ_ONCE(subreq->next_donated);
    139         if (prev_donated || next_donated) {
    140                 spin_lock_bh(&rreq->lock);
    141                 prev_donated = subreq->prev_donated;
    142                 next_donated =  subreq->next_donated;
    143                 subreq->start -= prev_donated;
    144                 subreq->len += prev_donated;
    145                 subreq->transferred += prev_donated;
    146                 prev_donated = subreq->prev_donated = 0;
    147                 if (subreq->transferred == subreq->len) {
    148                         subreq->len += next_donated;
    149                         subreq->transferred += next_donated;
    150                         next_donated = subreq->next_donated = 0;
    151                 }
    152                 trace_netfs_sreq(subreq, netfs_sreq_trace_add_donations);
    153                 spin_unlock_bh(&rreq->lock);
    154         }
    155 
    156         avail = subreq->transferred;
    157         if (avail == subreq->len)
    158                 avail += next_donated;
    159         start = subreq->start;
    160         if (subreq->consumed == 0) {
    161                 start -= prev_donated;
    162                 avail += prev_donated;
    163         } else {
    164                 start += subreq->consumed;
    165                 avail -= subreq->consumed;
    166         }
    167         part = umin(avail, fsize);
    168 
    169         trace_netfs_progress(subreq, start, avail, part);
    170 
    171         if (start + avail >= fend) {
    172                 if (fpos == start) {
    173                         /* Flush, unlock and mark for caching any folio we've just read. */
    174                         subreq->consumed = fend - subreq->start;
--> 175                         netfs_unlock_read_folio(subreq, rreq, folioq, slot);
                                                                      ^^^^^^
Unchecked dereference

    176                         folioq_mark2(folioq, slot);
                                             ^^^^^^

    177                         if (subreq->consumed >= subreq->len)
    178                                 goto remove_subreq;
    179                 } else if (fpos < start) {
    180                         excess = fend - subreq->start;
    181 
    182                         spin_lock_bh(&rreq->lock);

regards,
dan carpenter