Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1649933

Re: [PATCHv2 0/3] Enable no_cache flag to driver_data

Show all headers | View raw

On Wed, May 24, 2017 at 09:03:57PM +0200, Luis R. Rodriguez wrote:
> __fw_lookup_buf() really should be
> renamed to something that reflects this is a cache lookup.

Actually I take this back, other than the cache, note that when we
fw_lookup_and_allocate_buf() we first __fw_lookup_buf() but if the buf is not
there we allocate a new one and list_add() it to the cache regardless of
whether or not the cache thing has been enabled.

What this does then *also* other than caching for suspend/resume (which we
should document more formally) is to gather up contending lookups together
with one buf, and share the final status of just one lookup. If a buf is
found we fw_state_wait() on _request_firmware_prepare() until the buf clears.

John Ewalt recently reported some issues with loadng multiple files at the
same time. He also provided a patch. The swake_up() fix seems sensible
and would seem to have been caused by the swait transformation, but in
inspecting the other proposed changes it would seem we have had tons of
other lingering bugs which have probably existed for ages.

For instance, if there are pending requests for a leader request to send back
info, and one is about to complete but in the last moment on
assign_firmware_buf() fails, all the error paths lack a wake up call. As such
all pending requests may just wait and linger, and since none of these have
a timeout I would expect these to just linger forever. I'm not even sure we
kref_get() properly on the buf for pending requests when we are waiting for
a serialized request, ie, we might be able to take a buf underneath the nose
of a waiter.

Although some are new bugs, some seem to be really old bugs.

These are the sorts of issues I wish for a test driver to be able to uncover,
test and ensure we never regress again. This is also why I am being careful
about enabling a feature, we should *really* think things through well before
enabling on the new API.

[0] https://bugzilla.kernel.org/show_bug.cgi?id=195477

  Luis

Back to linux.kernel | Previous | Next — Previous in thread | Next in thread | Find similar | Unroll thread

Thread

[PATCHv2 0/3] Enable no_cache flag to driver_data  yi1.li@linux.intel.com - 2017-05-20 09:00 +0200
  [PATCHv2 1/3] firmware_class: move NO_CACHE from private to driver_data_req_params yi1.li@linux.intel.com - 2017-05-20 09:00 +0200
  Re: [PATCHv2 0/3] Enable no_cache flag to driver_data "Luis R. Rodriguez" <mcgrof@kernel.org> - 2017-05-24 21:10 +0200
    Re: [PATCHv2 0/3] Enable no_cache flag to driver_data "Luis R. Rodriguez" <mcgrof@kernel.org> - 2017-05-24 22:40 +0200
    Re: [PATCHv2 0/3] Enable no_cache flag to driver_data "Li, Yi" <yi1.li@linux.intel.com> - 2017-05-26 00:40 +0200
      Re: [PATCHv2 0/3] Enable no_cache flag to driver_data "Luis R. Rodriguez" <mcgrof@kernel.org> - 2017-05-26 00:50 +0200
        Re: [PATCHv2 0/3] Enable no_cache flag to driver_data "Luis R. Rodriguez" <mcgrof@kernel.org> - 2017-05-27 03:30 +0200
        Re: [PATCHv2 0/3] Enable no_cache flag to driver_data "Li, Yi" <yi1.li@linux.intel.com> - 2017-05-27 03:30 +0200
    Re: [PATCHv2 0/3] Enable no_cache flag to driver_data "Li, Yi" <yi1.li@linux.intel.com> - 2017-06-06 21:40 +0200

csiph-web