Re: [PATCH] crypto/ccp: don't disable interrupts while setting up debugfs

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On 2/25/2018 8:04 PM, Hook, Gary wrote:
> On 2/23/2018 5:33 PM, Sebastian Andrzej Siewior wrote:
>> I don't why we need take a single write lock and disable interrupts
>> while setting up debugfs. This is what what happens when we try anyway:
> 
> There is more than one CCP on some processors. The lock is intended to
> serialize attempts to initialize the new directory, but a R/W lock isn't
> required.
> 
> My testing on  an EPYC (8 CCPs) didn't expose this problem. May I ask what
> hardware you used here?

Probably not a hardware issue as opposed to a kernel configuration. Try
using CONFIG_DEBUG_ATOMIC_SLEEP and see if you can recreate.  And if irqs
are disabled, then you're probably looking at having to use a spinlock to
serialize creation of the directory.

Thanks,
Tom

> 
>> |ccp 0000:03:00.2: enabling device (0000 -> 0002)
>> |BUG: sleeping function called from invalid context at
>> kernel/locking/rwsem.c:69
>> |in_atomic(): 1, irqs_disabled(): 1, pid: 3, name: kworker/0:0
>> |irq event stamp: 17150
>> |hardirqs last  enabled at (17149): [<0000000097a18c49>]
>> restore_regs_and_return_to_kernel+0x0/0x23
>> |hardirqs last disabled at (17150): [<000000000773b3a9>]
>> _raw_write_lock_irqsave+0x1b/0x50
>> |softirqs last  enabled at (17148): [<0000000064d56155>]
>> __do_softirq+0x3b8/0x4c1
>> |softirqs last disabled at (17125): [<0000000092633c18>] irq_exit+0xb1/0xc0
>> |CPU: 0 PID: 3 Comm: kworker/0:0 Not tainted 4.16.0-rc2+ #30
>> |Workqueue: events work_for_cpu_fn
>> |Call Trace:
>> | dump_stack+0x7d/0xb6
>> | ___might_sleep+0x1eb/0x250
>> | down_write+0x17/0x60
>> | start_creating+0x4c/0xe0
>> | debugfs_create_dir+0x9/0x100
>> | ccp5_debugfs_setup+0x191/0x1b0
>> | ccp5_init+0x8a7/0x8c0
>> | ccp_dev_init+0xb8/0xe0
>> | sp_init+0x6c/0x90
>> | sp_pci_probe+0x26e/0x590
>> | local_pci_probe+0x3f/0x90
>> | work_for_cpu_fn+0x11/0x20
>> | process_one_work+0x1ff/0x650
>> | worker_thread+0x1d4/0x3a0
>> | kthread+0xfe/0x130
>> | ret_from_fork+0x27/0x50
>>
>> If any locking is required, a simple mutex will do it.
>>
>> Cc: Gary R Hook <gary.hook@xxxxxxx>
>> Signed-off-by: Sebastian Andrzej Siewior <bigeasy@xxxxxxxxxxxxx>
>> ---
>>   drivers/crypto/ccp/ccp-debugfs.c | 7 +++----
>>   1 file changed, 3 insertions(+), 4 deletions(-)
>>
>> diff --git a/drivers/crypto/ccp/ccp-debugfs.c
>> b/drivers/crypto/ccp/ccp-debugfs.c
>> index 59d4ca4e72d8..1a734bd2070a 100644
>> --- a/drivers/crypto/ccp/ccp-debugfs.c
>> +++ b/drivers/crypto/ccp/ccp-debugfs.c
>> @@ -278,7 +278,7 @@ static const struct file_operations
>> ccp_debugfs_stats_ops = {
>>   };
>>     static struct dentry *ccp_debugfs_dir;
>> -static DEFINE_RWLOCK(ccp_debugfs_lock);
>> +static DEFINE_MUTEX(ccp_debugfs_lock);
>>     #define    MAX_NAME_LEN    20
>>   @@ -290,16 +290,15 @@ void ccp5_debugfs_setup(struct ccp_device *ccp)
>>       struct dentry *debugfs_stats;
>>       struct dentry *debugfs_q_instance;
>>       struct dentry *debugfs_q_stats;
>> -    unsigned long flags;
>>       int i;
>>         if (!debugfs_initialized())
>>           return;
>>   -    write_lock_irqsave(&ccp_debugfs_lock, flags);
>> +    mutex_lock(&ccp_debugfs_lock);
>>       if (!ccp_debugfs_dir)
>>           ccp_debugfs_dir = debugfs_create_dir(KBUILD_MODNAME, NULL);
>> -    write_unlock_irqrestore(&ccp_debugfs_lock, flags);
>> +    mutex_unlock(&ccp_debugfs_lock);
>>       if (!ccp_debugfs_dir)
>>           return;
>>  
> 



[Index of Archives]     [Kernel]     [Gnu Classpath]     [Gnu Crypto]     [DM Crypt]     [Netfilter]     [Bugtraq]

  Powered by Linux