Shyam Prasad N <nspmangalore@xxxxxxxxx> writes:
> On Wed, Mar 15, 2023 at 3:49 AM Paulo Alcantara <pc@xxxxxxxxxxxxx> wrote:
>> Shyam Prasad N <nspmangalore@xxxxxxxxx> writes:
>>
>> > After cifs_tree_connect, tcon status should not be
>> > set to TID_GOOD. There could still be files that need
>> > reopen. The status should instead be changed to
>> > TID_NEED_FILES_INVALIDATE. That way, after reopen of
>> > files, the status can be changed to TID_GOOD.
>> >
>> > Signed-off-by: Shyam Prasad N <sprasad@xxxxxxxxxxxxx>
>> > ---
>> > fs/cifs/cifsglob.h | 2 +-
>> > fs/cifs/connect.c | 14 ++++++++++----
>> > fs/cifs/dfs.c | 16 +++++++++++-----
>> > fs/cifs/file.c | 10 +++++-----
>> > 4 files changed, 27 insertions(+), 15 deletions(-)
>>
>> With this patch, the status of TID_GOOD is no longer set after
>> reconnecting tcons. I've noticed that when the DFS cache refresher
>> attempted to get a new referral for updating a cached entry but the IPC
>> tcon status was still set to TID_NEED_FILES_INVALIDATE, therefore
>> skipping the I/O as it thought the IPC tcon was disconnected.
>>
>> IOW, the TID_NEED_FILES_INVALIDATE status remains set for both types of
>> tcons after reconnect.
>>
>> Besides, could you please split this patch into two? The first one
>> would fix the checking of tcon statuses by using the appropriate spin
>> lock, and the second would make use of TID_NEED_FILES_INVALIDATE on
>> non-IPC tcons and set the TID_GOOD status afterwards.
>
> Good point. The tcon status were indeed messed up after this patch.
> Should have checked it earlier. My bad.
>
> Let me revert this one and include only the checking of tcon status
> with the right lock.
>
> Attached the patch for that.
Thanks for the patch! Looks good to me.
BTW, don't you think we need to get rid of unnecessary ses status check
in __refresh_tcon() as well
...
spin_lock(&ipc->tc_lock);
if (ses->ses_status != SES_GOOD || ipc->status != TID_GOOD) {
