can_populate_pages() in lustre/llite/vvp_io.c restarts a read or a write
if the cached layout generation is not the same as the generation that the
IO recorded at cl_io_init() time:

                if (ll_layout_version_get(lli) != vio->vui_layout_gen ||
                    CFS_FAIL_CHECK_QUIET(OBD_FAIL_LLITE_LOST_LAYOUT)) {
                        io->ci_need_restart = 1;
                        /* this will cause a short read/write */
                        io->ci_continue = 0;
                        rc = false;
                }

Three sites write lli_layout_gen:

1. llite_lib.c:1440 - the client initializes the inode.
2. vvp_object.c:119 - the client discards the cached layout. This site writes the CL_LAYOUT_GEN_NONE sentinel.
3. file.c:6827 - the client applies a layout that it fetched.

The client cannot see the difference between a layout that changed and a
layout that the client no longer keeps in its cache. Two events write site
2. A remote layout change that revokes the lock writes it. The LDLM LRU
shrinker of the client also writes it when the shrinker cancels the lock
during memory reclaim. After the second event, the layout is the same
before and after the cancel, but the IO restarts.

Why the loop does not stop

One restart is not a problem. The problem is that no mechanism limits the
loop. ll_layout_refresh() does not keep the layout lock. Its own comment
gives this warning:

"This function will not hold layout lock so it may be revoked any time
after this function returns."

ll_layout_lock_set() releases the reference in all conditions at
file.c:6971. The window between the refresh and the can_populate_pages()
check is therefore open to the next cancel. If memory reclaim continues,
the client discards the layout again at each attempt. The IO then uses all
of the RETRY_ATTEMPTS budget of 1000 in ll_file_io_generic(), and it makes
no progress.

The check also runs more than one time in a single cl_io.
lov_io_rw_advance() sets io->ci_continue to next < lio->lis_io_endpos at
lov_io.c:1270. cl_io_loop() then enters cio_start again. Thus
can_populate_pages() runs one time for each stripe extent. The
cl_io_init() window is not the only exposure.

Field data

These data come from a soak run. The load was buffered fio on a client at version 2.14.0.

• The failing IO did 1001 restarts and used all of the budget.
• The layout did not change. dump_lsm gave the same layout_gen of 3 in
  1614 of 1694 iterations.
• All 106 calls to vvp_io_write_start() from the failing thread stopped
  at the can_populate_pages() return.
• The client sent all of the cancels. The captures hold 71733 server
  blocking callbacks in 15 samples, and none of them is for a layout
  lock. The same captures hold 7179 client-side layout cancels on the
  failing files.
• The shrinker was a second application thread on the same node in
  direct reclaim. That thread made 13301 calls to ldlm_pool_shrink() in
  14.4 s.
• The lock was a correct LRU candidate. Its reference counts were
  lrc: 2/0,0 in 2018 of 2027 cancels, because ll_layout_refresh()
  releases its reference by design.
• The window between the refresh and the check is approximately 0.4 ms.
  The shrinker entered that window in 85 of 106 attempts.

Relation to LU-20589

LU-20589 holds the result that the user sees when the IO uses all of the
budget: a buffered write(2) returns 0. This ticket holds the restart
itself. A correction to the restart removes the livelock. LU-20589 only
makes the livelock visible.

What the check protects

The comment above can_populate_pages() tells you that the check is safe
because a layout swap must hold the group lock. That statement is not
correct:

• ll_swap_layouts() takes the group lock only if user space gives a gid
  that is not zero (file.c:3718).
• ll_swap_layouts_close() (file.c:1588) takes no group lock. This is the
  path that lfs migrate and HSM use.

lo_active_ios is the mechanism that keeps an IO from putting pages onto
the wrong stripe. lov_conf_set() returns -EBUSY while an IO is active
(lov_object.c:1512). The client holds the count for the full span from
cl_io_init() to cl_io_fini().

The generation check is still necessary. __cl_io_init() walks the slice
stack from the top down. vvp_io_init() records vui_layout_gen
(vvp_io.c:1878) before lov_io_init() pins the LSM (lov_io.c:2192). A swap
in that gap is a real change, and only this check finds it.

Why you cannot make the check more narrow

The first idea is to read CL_LAYOUT_GEN_NONE as "the client discarded the
cache" and not as "the layout changed", and then to restart only for a
known generation that is different. That idea is wrong. Record the reason,
so that no one tries it again.

The sentinel is the only signal in an IO that a real layout change exists:

• A remote layout change comes to the client first, and only, as
  OBJECT_CONF_INVALIDATE. That path sets LO_LAYOUT_INVALID and returns
  without a change to lo_lsm (lov_object.c:1460). lsm_layout_gen thus
  keeps the old value. No reader on the IO path examines
  LO_LAYOUT_INVALID.
• The client fetches the new generation only when it must.
  ll_layout_refresh() returns immediately unless the cached generation
  is already the sentinel (file.c:7092). ll_layout_lock_set() is the
  only site that writes a real generation, and ll_layout_refresh() is
  its only caller (file.c:7112).

The sentinel is therefore a precondition for the client to learn the new
generation. It is not an alternative to the new generation. If you ignore
the sentinel, you delete the detection. You do not make it more narrow.
Change 38020 removed the check, and reads crashed.

A comparison against a generation that you get in place has the same
fault. cl_object_layout_get() (lov_object.c:2293) is an example. Its value
comes from lo_lsm, and the invalidate leaves lo_lsm stale on purpose.

You also cannot make the decision from the type of the cancel. The client
enters ll_lock_cancel_bits() in the same way from ldlm_cancel_lru() and
from ll_md_blocking_ast(). No state shows which one called it.

A correct comparison needs a refreshed generation. You cannot do a refresh
from can_populate_pages(), because that function runs with the OST extent
lock, lli_trunc_sem and the range lock held. vvp_io_fini() (vvp_io.c:344
and vvp_io.c:388) shows a shape that works. It compares vui_layout_gen
against the generation that a new ll_layout_refresh() returns, at a point
where it holds no extent lock.

Directions that remain

Two directions remain. Neither of them changes the condition.

• Compare the content of the refreshed layout, and not its generation.
  Then a discard and a re-fetch of the same layout does not restart the
  IO.
• Keep voluntary LRU reclaim from taking a layout lock that supports a
  live IO. Apply this to LRU reclaim only, and never to a server
  blocking callback. If you defer a real callback, you close a wait-for
  cycle with lfs migrate --block, which holds an OST group lock across
  MDS_SWAP_LAYOUTS. Also note that a layout lock is not a separate lock.
  The MDT grants MDS_INODELOCK_LAYOUT together with LOOKUP, UPDATE, PERM
  and DOM in one ibits lock (mdt_open.c:927 and 951-959), and LDLM has
  no reference for a single bit. A reader reference that you hold for
  the life of an IO therefore also delays operations on the same file
  that have no relation to the layout.

Related history

• LU-2017 (4af3ab1945) added can_populate_pages() and
  CL_LAYOUT_GEN_NONE in one commit. It did not examine the interaction
  between them.
• LU-6389 found the same mechanism ("File layout as not changed and when
  lock is taken back, layout_gen is still 0") and corrected the other
  side of it. It also records the basic difficulty: "the client doesn't
  know if the layout is still the same."
• Change 14123 asked the question in review. A reviewer wrote that the
  client can cancel the LAYOUT bit without a change to the layout, and
  that the client must not set ci_need_restart in that condition. The
  gap has a name, but no one closed it.
• Change 5866 ("don't set lli_layout_gen unnecessarily") is the only
  change with the shape of this fix. Its author abandoned it and did not
  send a new patch set.
• Change 38020 removed the check. Its author abandoned it, because reads
  crash without the check.
• LU-14760 (change 43995) set RETRY_ATTEMPTS to 1000 to put a limit on a
  loop that "is stuck forever". No one gave a reason for that value.
• LU-18416 and LU-18435 report the same behavior, 1000 restarts and a
  write that returns 0, from a trigger on the server. Both have a
  correction on the server.