2011/6/9 Christoph Hellwig <hch@xxxxxxxxxxxxx>:
>> static void
>> cifs_put_super(struct super_block *sb)
>> {
>> + struct cifs_sb_info *cifs_sb = CIFS_SB(sb);
>> + if (cifs_sb == NULL) {
>> + cFYI(1, "Empty cifs superblock info passed to put_super");
>> + return;
>> + }
>> +
>> + bdi_destroy(&cifs_sb->bdi);
>
> This means you have a problem with the lifetime rules in cifs_do_mount.
>
> The VFS only calls ->put_super if sb->s_root is set. So the rules for
> the mount handler are to only set s_root once the superblock is fully
> set up.
>
> Also you should never call cifs_umount from the error handling in
> cifs_do_mount. Until s_root is set, please unwind manually, after
> that leave it to ->put_super.
>
>
>> +
>> +static void
>> +cifs_kill_super(struct super_block *sb)
>> +{
>
> This also seems quite broken. If you fix up the mount path like
> I suggested it won't be nessecary.
>
>> int rc = 0;
>> struct cifs_sb_info *cifs_sb;
>>
>> cFYI(1, "In cifs_put_super");
>> cifs_sb = CIFS_SB(sb);
>> if (cifs_sb == NULL) {
>
> And this check should also be removed.
>
>
It seems to me that I didn't understand your points clearly.
That's how I reproduce the problem:
1) one process does mount/umount calls in a loop;
2) another process does mount and umount calls.
As the result, kernel crashes sometimes on mount of the second
process, sometimes - on umount of the second process. The problem is
that:
one of these two processes process umount call. So, it comes to
cifs_put_super, calls cifs_umount and then calls kfree(cifs_sb). In
the same time, another process comes into cifs_do_mount and calls
sget(). Then it appers into cifs_match_super that thinks that all
structures like server, ses, tcon and cifs_sb are valid, but it is not
true - the first process has already freed them but didn't remove
superblock from fs_supers list.
So, I simply reodered calls: now umount codepath marks a superblock as
non-active and calls kill_sb(new cifs_kill_super), it calls
kill_anon_super (that calls cifs_put_super and removes superblock from
fs_supers list) and after that kill_sb frees all cifs structures
(server, ses, tcon, cifs_sb).
As the result, when we get into cifs_match_super (in mount from
another process) we are sure that all cifs structures are valid.
If I am not right, correct me, please!
--
Best regards,
Pavel Shilovsky.
--
To unsubscribe from this list: send the line "unsubscribe linux-cifs" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
