Re: [RFC PATCH] sys_read: add a compat_sys_read for 64bit system
On 2016/6/10 1:08, Andy Lutomirski wrote:
> On Tue, Jun 7, 2016 at 7:14 PM, Zhangjian (Bamvor)
> wrote:
>> Hi,
>>
>> On 2016/6/8 9:33, Weidong Wang wrote:
>>>
>>> Test 32 progress and 64 progress on the 64bit system with
>>> this progress:
>>>
>>> int main(int argc, char **argv)
>>> {
>>> int fd = 0;
>>> int i, ret = 0;
>>> char buf[512];
>>> unsigned long count = -1;
>>>
>>> fd = open("/tmp", O_RDONLY);
>>> if (fd < -1) {
>>> printf("Pls check the directory is exist?\n");
>>> return -1;
>>> }
>>> errno = 0;
>>> ret = read(fd, NULL, count);
>>> printf("Ret is %d errno %d\n", ret, errno);
>>> close(fd);
>>>
>>> return 0;
>>> }
>>>
>>> we get the different errno. The 64 progress we get errno is -14 while
>>> the 32 progress is -21.
>
> On 64-bit, you get -14 == -EFAULT. Seems reasonable: you passed a bad
> pointer.
>
> On 32-bit, you get -21 == -EISDIR. Also seems reasonable: fd is a directory.
>
>>>
>>> The reason is that, the user progress would use a 32bit count, while
>>> the sys_read size_t in kernel is 64bit. When the uesrspace count is
>>> -1(0x), it goes to the sys_read, it would be change to a positive
>>> number.
>
> That parameter is size_t, which is unsigned. It's a positive number
> in both cases.
>
> I don't think there's a bug here.
>
Yep.
In the progress open the '/tmp' is a directory. If we do open a file
'/tmp/files' (exist file),
the result would be different on x86-64bit machine.
On 64-bit, we get -14 == -EFAULT.
On 32-bit, we get the length of the file, the errno is 0.
Regards,
Weidong
> .
>
[RFC PATCH] sys_read: add a compat_sys_read for 64bit system
Test 32 progress and 64 progress on the 64bit system with
this progress:
int main(int argc, char **argv)
{
int fd = 0;
int i, ret = 0;
char buf[512];
unsigned long count = -1;
fd = open("/tmp", O_RDONLY);
if (fd < -1) {
printf("Pls check the directory is exist?\n");
return -1;
}
errno = 0;
ret = read(fd, NULL, count);
printf("Ret is %d errno %d\n", ret, errno);
close(fd);
return 0;
}
we get the different errno. The 64 progress we get errno is -14 while
the 32 progress is -21.
The reason is that, the user progress would use a 32bit count, while
the sys_read size_t in kernel is 64bit. When the uesrspace count is
-1(0x), it goes to the sys_read, it would be change to a positive
number.
So I think we should add a compat_sys_read for the read syscall. I test it
on x86 or arm64 platform. The patch works well.
As well this patch may do work for the 'tile' 64 system.
I think it may enter the same result on mips/parisc/powerpc/sparc.
The s390 do the compat_sys_s390_read for the compat sys_read.
Signed-off-by: Weidong Wang
---
arch/x86/entry/syscalls/syscall_32.tbl | 2 +-
fs/read_write.c| 8
include/linux/compat.h | 2 ++
include/uapi/asm-generic/unistd.h | 2 +-
4 files changed, 12 insertions(+), 2 deletions(-)
diff --git a/arch/x86/entry/syscalls/syscall_32.tbl
b/arch/x86/entry/syscalls/syscall_32.tbl
index 4cddd17..ebc24e3 100644
--- a/arch/x86/entry/syscalls/syscall_32.tbl
+++ b/arch/x86/entry/syscalls/syscall_32.tbl
@@ -9,7 +9,7 @@
0 i386restart_syscall sys_restart_syscall
1 i386exitsys_exit
2 i386forksys_forksys_fork
-3 i386readsys_read
+3 i386readsys_read
compat_sys_read
4 i386write sys_write
5 i386opensys_open
compat_sys_open
6 i386close sys_close
diff --git a/fs/read_write.c b/fs/read_write.c
index 933b53a..d244848 100644
--- a/fs/read_write.c
+++ b/fs/read_write.c
@@ -613,6 +613,14 @@ SYSCALL_DEFINE3(write, unsigned int, fd, const char __user
*, buf,
return ret;
}
+#ifdef CONFIG_COMPAT
+COMPAT_SYSCALL_DEFINE3(read, unsigned int, fd, char __user *, buf,
+ compat_size_t, count)
+{
+return sys_read(fd, buf, (compat_ssize_t)count);
+}
+#endif
+
SYSCALL_DEFINE4(pread64, unsigned int, fd, char __user *, buf,
size_t, count, loff_t, pos)
{
diff --git a/include/linux/compat.h b/include/linux/compat.h
index f964ef7..d88ccad 100644
--- a/include/linux/compat.h
+++ b/include/linux/compat.h
@@ -332,6 +332,8 @@ asmlinkage long compat_sys_keyctl(u32 option,
u32 arg2, u32 arg3, u32 arg4, u32 arg5);
asmlinkage long compat_sys_ustat(unsigned dev, struct compat_ustat __user
*u32);
+asmlinkage ssize_t compat_sys_read(unsigned int fd,
+ char __user * buf, compat_size_t count);
asmlinkage ssize_t compat_sys_readv(compat_ulong_t fd,
const struct compat_iovec __user *vec, compat_ulong_t vlen);
asmlinkage ssize_t compat_sys_writev(compat_ulong_t fd,
diff --git a/include/uapi/asm-generic/unistd.h
b/include/uapi/asm-generic/unistd.h
index a26415b..745818a 100644
--- a/include/uapi/asm-generic/unistd.h
+++ b/include/uapi/asm-generic/unistd.h
@@ -201,7 +201,7 @@ __SC_COMP(__NR_getdents64, sys_getdents64,
compat_sys_getdents64)
#define __NR3264_lseek 62
__SC_3264(__NR3264_lseek, sys_llseek, sys_lseek)
#define __NR_read 63
-__SYSCALL(__NR_read, sys_read)
+__SC_COMP(__NR_read, sys_read, compat_sys_read)
#define __NR_write 64
__SYSCALL(__NR_write, sys_write)
#define __NR_readv 65
--
2.7.0
[PATCH net-next] phy: make some bits preserved while setup forced mode
When tested the PHY SGMII Loopback:
1.set the LOOPBACK bit,
2.set the autoneg to AUTONEG_DISABLE, it calls the
genphy_setup_forced which will clear the bit.
The BMCR_LOOPBACK bit should be preserved.
As Florian pointed out that other bits should be preserved too.
So I make the BMCR_ISOLATE and BMCR_PDOWN as well.
Signed-off-by: Weidong Wang
---
the patch is evoluted from "phy: keep the BCMR_LOOPBACK bit
while setup forced mode".
---
drivers/net/phy/phy_device.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/drivers/net/phy/phy_device.c b/drivers/net/phy/phy_device.c
index d551df6..9b24d7e 100644
--- a/drivers/net/phy/phy_device.c
+++ b/drivers/net/phy/phy_device.c
@@ -818,8 +818,9 @@ static int genphy_config_advert(struct phy_device *phydev)
*/
int genphy_setup_forced(struct phy_device *phydev)
{
- int ctl = 0;
+ int ctl = phy_read(phydev, MII_BMCR);
+ ctl &= BMCR_LOOPBACK | BMCR_ISOLATE | BMCR_PDOWN;
phydev->pause = 0;
phydev->asym_pause = 0;
--
2.7.0
Re: [RESEND PATCH net-next] phy: keep the BCMR_LOOPBACK bit while setup forced mode
On 2016/4/14 2:41, Florian Fainelli wrote:
> On 13/04/16 04:59, Weidong Wang wrote:
>> When tested the PHY SGMII Loopback,:
>> 1.set the LOOPBACK bit,
>> 2.set the autoneg to AUTONEG_DISABLE, it calls the
>> genphy_setup_forced which will clear the bit.
>>
>> So just keep the LOOPBACK bit while setup forced mode.
>
> Humm, it makes sense why we want this one, but maybe we want other bits
> to be preserved too, like MII_ISOLATE for instance?
>
I will add the MII_ISOLATE as well.
> Or maybe we should have a separate way to put the PHY into loopback mode
> which is deterministic and takes care of forcing the link at the same time?
>
I test with the loopback mode, and the link status is undeterminable.
>>
>> Signed-off-by: Weidong Wang
>> ---
>> drivers/net/phy/phy_device.c | 2 ++
>> 1 file changed, 2 insertions(+)
>>
>> diff --git a/drivers/net/phy/phy_device.c b/drivers/net/phy/phy_device.c
>> index e551f3a..8da4b80 100644
>> --- a/drivers/net/phy/phy_device.c
>> +++ b/drivers/net/phy/phy_device.c
>> @@ -1124,7 +1124,9 @@ static int genphy_config_advert(struct phy_device
>> *phydev)
>> int genphy_setup_forced(struct phy_device *phydev)
>> {
>> int ctl = 0;
>> +int val = phy_read(phydev, MII_BMCR);
>>
>> +ctl |= val & BMCR_LOOPBACK;
>> phydev->pause = 0;
>> phydev->asym_pause = 0;
>>
>> -- 2.7.0
>>
>
>
Re: [RESEND PATCH net-next] phy: keep the BCMR_LOOPBACK bit while setup forced mode
On 2016/4/13 22:19, Sergei Shtylyov wrote:
> Hello.
>
> On 4/13/2016 2:59 PM, Weidong Wang wrote:
>
>> When tested the PHY SGMII Loopback,:
>> 1.set the LOOPBACK bit,
>> 2.set the autoneg to AUTONEG_DISABLE, it calls the
>> genphy_setup_forced which will clear the bit.
>>
>> So just keep the LOOPBACK bit while setup forced mode.
>>
>> Signed-off-by: Weidong Wang
>> ---
>> drivers/net/phy/phy_device.c | 2 ++
>> 1 file changed, 2 insertions(+)
>>
>> diff --git a/drivers/net/phy/phy_device.c b/drivers/net/phy/phy_device.c
>> index e551f3a..8da4b80 100644
>> --- a/drivers/net/phy/phy_device.c
>> +++ b/drivers/net/phy/phy_device.c
>> @@ -1124,7 +1124,9 @@ static int genphy_config_advert(struct phy_device
>> *phydev)
>> int genphy_setup_forced(struct phy_device *phydev)
>> {
>> int ctl = 0;
>> +int val = phy_read(phydev, MII_BMCR);
>
>Please place this declaration first, DaveM prefers declarations to be
> sorted from longest to shortest.
>
>>
>> +ctl |= val & BMCR_LOOPBACK;
>
>Just =, removing the 'ctl' initializer, please.
>
> [...]
>
> MBR, Sergei
>
Got it.
Regards,
Weidong
>
>
[RESEND PATCH net-next] phy: keep the BCMR_LOOPBACK bit while setup forced mode
When tested the PHY SGMII Loopback,:
1.set the LOOPBACK bit,
2.set the autoneg to AUTONEG_DISABLE, it calls the
genphy_setup_forced which will clear the bit.
So just keep the LOOPBACK bit while setup forced mode.
Signed-off-by: Weidong Wang
---
drivers/net/phy/phy_device.c | 2 ++
1 file changed, 2 insertions(+)
diff --git a/drivers/net/phy/phy_device.c b/drivers/net/phy/phy_device.c
index e551f3a..8da4b80 100644
--- a/drivers/net/phy/phy_device.c
+++ b/drivers/net/phy/phy_device.c
@@ -1124,7 +1124,9 @@ static int genphy_config_advert(struct phy_device *phydev)
int genphy_setup_forced(struct phy_device *phydev)
{
int ctl = 0;
+ int val = phy_read(phydev, MII_BMCR);
+ ctl |= val & BMCR_LOOPBACK;
phydev->pause = 0;
phydev->asym_pause = 0;
-- 2.7.0
[PATCH net-next] phy: keep the BCMR_LOOPBACK bit while setup forced mode
When tested the PHY SGMII Loopback,:
1.set the LOOPBACK bit,
2.set the autoneg to AUTONEG_DISABLE, it calls the
genphy_setup_forced which will clear the bit.
So just keep the LOOPBACK bit while setup forced mode.
Signed-off-by: Weidong Wang
---
drivers/net/phy/phy_device.c | 2 ++
1 file changed, 2 insertions(+)
diff --git a/drivers/net/phy/phy_device.c b/drivers/net/phy/phy_device.c
index e551f3a..8da4b80 100644
--- a/drivers/net/phy/phy_device.c
+++ b/drivers/net/phy/phy_device.c
@@ -1124,7 +1124,9 @@ static int genphy_config_advert(struct phy_device *phydev)
int genphy_setup_forced(struct phy_device *phydev)
{
int ctl = 0;
+ int val = phy_read(phydev, MII_BMCR);
+ ctl |= val & BMCR_LOOPBACK;
phydev->pause = 0;
phydev->asym_pause = 0;
--
2.7.0
[Ask for help] met a deadlock with switch_fpu_finish on suse 3.0.93-0.8-default kernel
Hi all,
We find a deadlock problem in suse 3.0.93-0.8-default kernel when
restore_fpu_checking return error in task switch.
The Call Trace is :
193 PID: 2415 TASK: 880b739d24c0 CPU: 5 COMMAND: "qemu-kvm"
194 #0 [880c7f6a6e40] crash_nmi_callback at 8102460f
195 #1 [880c7f6a6e50] notifier_call_chain at 81465027
196 #2 [880c7f6a6e80] __atomic_notifier_call_chain at 8146506d
197 #3 [880c7f6a6e90] notify_die at 814650bd
198 #4 [880c7f6a6ec0] default_do_nmi at 81462507
199 #5 [880c7f6a6ee0] do_nmi at 81462738
200 #6 [880c7f6a6ef0] restart_nmi at 81461c91
201 [exception RIP: _raw_spin_lock+21]
202 RIP: 814611e5 RSP: 8809d8d1ba80 RFLAGS: 0093
203 RAX: 0010 RBX: 0010 RCX: 0093
204 RDX: 8809d8d1ba80 RSI: 0018 RDI: 0001
205 RBP: 814611e5 R8: 814611e5 R9: 0018
206 R10: 8809d8d1ba80 R11: 0093 R12:
207 R13: 880c7f6b0a00 R14: 0005 R15: e2b8
208 ORIG_RAX: e2b8 CS: 0010 SS: 0018
209 --- ---
210 #7 [8809d8d1ba80] _raw_spin_lock at 814611e5
211 #8 [8809d8d1ba80] try_to_wake_up at 81054afb
212 #9 [8809d8d1bad0] pollwake at 8116cfc6
213 #10 [8809d8d1bb10] __wake_up_common at 81046e1a
214 #11 [8809d8d1bb50] __wake_up at 8104bf43
215 #12 [8809d8d1bb90] __send_signal at 81074bfd
216 #13 [8809d8d1bbd0] force_sig_info at 81076194
217 #14 [8809d8d1bc00] __switch_to at 81001930
218 #15 [8809d8d1bcf0] reschedule_interrupt at 8146a06e
219 #16 [8809d8d1bd58] vmx_handle_external_intr at a03c3f4c
[kvm_intel]
220 #17 [8809d8d1bd80] vcpu_enter_guest at a0363487 [kvm]
221 #18 [8809d8d1be00] __vcpu_run at a0363743 [kvm]
222 #19 [8809d8d1be40] kvm_arch_vcpu_ioctl_run at a0364438 [kvm]
223 #20 [8809d8d1be70] kvm_vcpu_ioctl at a0350cee [kvm]
224 #21 [8809d8d1bf10] do_vfs_ioctl at 8116bd1b
225 #22 [8809d8d1bf40] sys_ioctl at 8116c0e1
226 #23 [8809d8d1bf80] system_call_fastpath at 81469172
We see the patch
commit 80ab6f1e8c981b1b6604b2f22e36c917526235cd
"i387: use 'restore_fpu_checking()' directly in task switching code"
this patch remove the __math_state_restore in switch_fpu_finish,like that:
static inline void switch_fpu_finish(struct task_struct *new, fpu_switch_t fpu)
{
- if (fpu.preload)
- __math_state_restore(new);
+ if (fpu.preload) {
+ if (unlikely(restore_fpu_checking(new)))
+ __thread_fpu_end(new);
+ }
}
So in switch_fpu_finish, when entered restore_fpu_checking fail, it won't call
force_sig().
1. Would it will fix this issuse(deadlock)?
2. We don't understand why the restore_fpu_checking would failed? Any one know
that?
3. if the patch can fix the problem, We want to know that
"restore_fpu_checking(tsk) really fail,and we not force send the SIGSEGV to
the task,
Would it introuduce other issue?"
Regards,
Weidong
Re: [PATCH net-next] netfilter: nf_conntrack: remove the unneed check for *bucket
On 2016/1/31 5:30, Florian Westphal wrote:
> Weidong Wang wrote:
>> In the 'for(...) {}', the *bucket alwasy < net->ct.htable_size,
>> so remove the check
>> @@ -1383,14 +1383,12 @@ get_next_corpse(struct net *net, int (*iter)(struct
>> nf_conn *i, void *data),
>> lockp = &nf_conntrack_locks[*bucket % CONNTRACK_LOCKS];
>> local_bh_disable();
>> spin_lock(lockp);
>> -if (*bucket < net->ct.htable_size) {
>
> AFAIU net->ct.htable_size can shrink between for-test and aquiring
> the bucket lockp, so this additional if-test is needed.
>
ok, Got it.
So ignore this patch.
Regards,
Weidong
> .
>
[PATCH net-next] netfilter: nf_conntrack: remove the unneed check for *bucket
In the 'for(...) {}', the *bucket alwasy < net->ct.htable_size,
so remove the check
Signed-off-by: Weidong Wang
---
net/netfilter/nf_conntrack_core.c | 14 ++
1 file changed, 6 insertions(+), 8 deletions(-)
diff --git a/net/netfilter/nf_conntrack_core.c
b/net/netfilter/nf_conntrack_core.c
index 3cb3cb8..cd7d5c8 100644
--- a/net/netfilter/nf_conntrack_core.c
+++ b/net/netfilter/nf_conntrack_core.c
@@ -1383,14 +1383,12 @@ get_next_corpse(struct net *net, int (*iter)(struct
nf_conn *i, void *data),
lockp = &nf_conntrack_locks[*bucket % CONNTRACK_LOCKS];
local_bh_disable();
spin_lock(lockp);
- if (*bucket < net->ct.htable_size) {
- hlist_nulls_for_each_entry(h, n,
&net->ct.hash[*bucket], hnnode) {
- if (NF_CT_DIRECTION(h) != IP_CT_DIR_ORIGINAL)
- continue;
- ct = nf_ct_tuplehash_to_ctrack(h);
- if (iter(ct, data))
- goto found;
- }
+ hlist_nulls_for_each_entry(h, n, &net->ct.hash[*bucket],
hnnode) {
+ if (NF_CT_DIRECTION(h) != IP_CT_DIR_ORIGINAL)
+ continue;
+ ct = nf_ct_tuplehash_to_ctrack(h);
+ if (iter(ct, data))
+ goto found;
}
spin_unlock(lockp);
local_bh_enable();
--
2.7.0
Re: [PATCH 1/3] megaraid_sas: Convert dev_printk to dev_
On 2015/10/27 18:33, Kashyap Desai wrote: >>> + dev_dbg(&instance->pdev->dev, "Error copying out >>> cmd_status\n"); >>> error = -EFAULT; >>> } >>> >> >> Reviewed-by: Johannes Thumshirn > > We will consider all three patches for future submission. As of now we have > two patch set pending to be committed. > We are working for few more patch in megaraid_sas which will do clean up in > driver module + proper error handling of DCMD command timeout. It will cover > Patch posted with below subject - > > [PATCH 3/3] megaraid_sas: return -ENOMEM when create DMA pool for cmd frames > failed > Ok. And that, can you add Signed-off-by with me as well? Regards, Weidong > James - We will be resending these patch set on top of latest outstanding > megaraid_sas driver patch, so that we can avoid any conflict in commits. > > -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 1/3] megaraid_sas: Convert dev_printk to dev_
On 2015/10/28 3:35, Joe Perches wrote:
> On Tue, 2015-10-27 at 16:26 +0800, Weidong Wang wrote:
>> Reduce object size a little by using dev_
>> calls instead of dev_printk(KERN_.
>
> This is also not the same output.
>
> dev_printk(KERN_DEBUG vs dev_dbg has the same
> behavior as printk(KERN_DEBUG vs pr_debug
>
yep, You are right.
As Kashyap said, these two patches(Covert [dev_]printk to [dev|pr]_) may
introduced conflicts to their working for megaraid_sas.
So just ignore these two patches.
Regards,
Weidong
>> diff --git a/drivers/scsi/megaraid/megaraid_sas_base.c
>> b/drivers/scsi/megaraid/megaraid_sas_base.c
> []
>> @@ -1884,7 +1884,7 @@ static int megasas_get_ld_vf_affiliation_111(struct
>> megasas_instance *instance,
>> cmd = megasas_get_cmd(instance);
>>
>> if (!cmd) {
>> -dev_printk(KERN_DEBUG, &instance->pdev->dev,
>> "megasas_get_ld_vf_affiliation_111:"
>> +dev_dbg(&instance->pdev->dev,
>> "megasas_get_ld_vf_affiliation_111:"
>> "Failed to get cmd for scsi%d\n",
>> instance->host->host_no);
>> return -ENOMEM;
> []
>> @@ -5243,7 +5243,7 @@ static int megasas_probe_one(struct pci_dev *pdev,
>> &instance->consumer_h);
>>
>> if (!instance->producer || !instance->consumer) {
>> -dev_printk(KERN_DEBUG, &pdev->dev, "Failed to allocate"
>> +dev_dbg(&pdev->dev, "Failed to allocate"
>> "memory for producer, consumer\n");
>
> Note the lack of a space between coalesced string segment words.
> That's one of the reasons to coalesce them.
>
>
>
> .
>
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
[PATCH 3/3] megaraid_sas: return -ENOMEM when create DMA pool for cmd frames failed
when create DMA pool for cmd frames failed, we should return -ENOMEM,
instead of 0.
In some case in:
megasas_init_adapter_fusion()
-->megasas_alloc_cmds()
-->megasas_create_frame_pool
create DMA pool failed,
--> megasas_free_cmds() [1]
-->megasas_alloc_cmds_fusion()
failed, then goto fail_alloc_cmds.
-->megasas_free_cmds() [2]
we will call megasas_free_cmds twice, [1] will kfree cmd_list,
[2] will use cmd_list.it will cause a problem:
Unable to handle kernel NULL pointer dereference at virtual address
pgd = ffc000f7
[] *pgd=001fbf893003, *pud=001fbf893003, *pmd=001fbf894003,
*pte=00606d000707
Internal error: Oops: 9605 [#1] SMP
Modules linked in:
CPU: 18 PID: 1 Comm: swapper/0 Not tainted
task: ffdfb929 ti: ffdfb923c000 task.ti: ffdfb923c000
PC is at megasas_free_cmds+0x30/0x70
LR is at megasas_free_cmds+0x24/0x70
...
Call trace:
[] megasas_free_cmds+0x30/0x70
[] megasas_init_adapter_fusion+0x2f4/0x4d8
[] megasas_init_fw+0x2dc/0x760
[] megasas_probe_one+0x3c0/0xcd8
[] local_pci_probe+0x4c/0xb4
[] pci_device_probe+0x11c/0x14c
[] driver_probe_device+0x1ec/0x430
[] __driver_attach+0xa8/0xb0
[] bus_for_each_dev+0x74/0xc8
[] driver_attach+0x28/0x34
[] bus_add_driver+0x16c/0x248
[] driver_register+0x6c/0x138
[] __pci_register_driver+0x5c/0x6c
[] megasas_init+0xc0/0x1a8
[] do_one_initcall+0xe8/0x1ec
[] kernel_init_freeable+0x1c8/0x284
[] kernel_init+0x1c/0xe4
Signed-off-by: Weidong Wang
---
drivers/scsi/megaraid/megaraid_sas_base.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/drivers/scsi/megaraid/megaraid_sas_base.c
b/drivers/scsi/megaraid/megaraid_sas_base.c
index 2287aa1..8215218 100644
--- a/drivers/scsi/megaraid/megaraid_sas_base.c
+++ b/drivers/scsi/megaraid/megaraid_sas_base.c
@@ -3746,8 +3746,9 @@ int megasas_alloc_cmds(struct megasas_instance *instance)
* Create a frame pool and assign one frame to each cmd
*/
if (megasas_create_frame_pool(instance)) {
- dev_dbg(&instance->pdev->dev, "Error creating frame DMA
pool\n");
+ dev_err(&instance->pdev->dev, "Error creating frame DMA
pool\n");
megasas_free_cmds(instance);
+ return -ENOMEM;
}
return 0;
--
1.9.0
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
[PATCH 1/3] megaraid_sas: Convert dev_printk to dev_
Reduce object size a little by using dev_
calls instead of dev_printk(KERN_.
Signed-off-by: Weidong Wang
---
drivers/scsi/megaraid/megaraid_sas_base.c | 68 +++
1 file changed, 34 insertions(+), 34 deletions(-)
diff --git a/drivers/scsi/megaraid/megaraid_sas_base.c
b/drivers/scsi/megaraid/megaraid_sas_base.c
index eaa81e5..ed9846d 100644
--- a/drivers/scsi/megaraid/megaraid_sas_base.c
+++ b/drivers/scsi/megaraid/megaraid_sas_base.c
@@ -1884,7 +1884,7 @@ static int megasas_get_ld_vf_affiliation_111(struct
megasas_instance *instance,
cmd = megasas_get_cmd(instance);
if (!cmd) {
- dev_printk(KERN_DEBUG, &instance->pdev->dev,
"megasas_get_ld_vf_affiliation_111:"
+ dev_dbg(&instance->pdev->dev,
"megasas_get_ld_vf_affiliation_111:"
"Failed to get cmd for scsi%d\n",
instance->host->host_no);
return -ENOMEM;
@@ -1908,7 +1908,7 @@ static int megasas_get_ld_vf_affiliation_111(struct
megasas_instance *instance,
sizeof(struct
MR_LD_VF_AFFILIATION_111),
&new_affiliation_111_h);
if (!new_affiliation_111) {
- dev_printk(KERN_DEBUG, &instance->pdev->dev, "SR-IOV:
Couldn't allocate "
+ dev_dbg(&instance->pdev->dev, "SR-IOV: Couldn't
allocate "
"memory for new affiliation for scsi%d\n",
instance->host->host_no);
megasas_return_cmd(instance, cmd);
@@ -1995,7 +1995,7 @@ static int megasas_get_ld_vf_affiliation_12(struct
megasas_instance *instance,
cmd = megasas_get_cmd(instance);
if (!cmd) {
- dev_printk(KERN_DEBUG, &instance->pdev->dev,
"megasas_get_ld_vf_affiliation12: "
+ dev_dbg(&instance->pdev->dev, "megasas_get_ld_vf_affiliation12:
"
"Failed to get cmd for scsi%d\n",
instance->host->host_no);
return -ENOMEM;
@@ -2020,7 +2020,7 @@ static int megasas_get_ld_vf_affiliation_12(struct
megasas_instance *instance,
sizeof(struct
MR_LD_VF_AFFILIATION),
&new_affiliation_h);
if (!new_affiliation) {
- dev_printk(KERN_DEBUG, &instance->pdev->dev, "SR-IOV:
Couldn't allocate "
+ dev_dbg(&instance->pdev->dev, "SR-IOV: Couldn't
allocate "
"memory for new affiliation for scsi%d\n",
instance->host->host_no);
megasas_return_cmd(instance, cmd);
@@ -2174,7 +2174,7 @@ int megasas_sriov_start_heartbeat(struct megasas_instance
*instance,
cmd = megasas_get_cmd(instance);
if (!cmd) {
- dev_printk(KERN_DEBUG, &instance->pdev->dev,
"megasas_sriov_start_heartbeat: "
+ dev_dbg(&instance->pdev->dev, "megasas_sriov_start_heartbeat: "
"Failed to get cmd for scsi%d\n",
instance->host->host_no);
return -ENOMEM;
@@ -2188,7 +2188,7 @@ int megasas_sriov_start_heartbeat(struct megasas_instance
*instance,
sizeof(struct
MR_CTRL_HB_HOST_MEM),
&instance->hb_host_mem_h);
if (!instance->hb_host_mem) {
- dev_printk(KERN_DEBUG, &instance->pdev->dev, "SR-IOV:
Couldn't allocate"
+ dev_dbg(&instance->pdev->dev, "SR-IOV: Couldn't
allocate"
" memory for heartbeat host memory for scsi%d\n",
instance->host->host_no);
retval = -ENOMEM;
@@ -2922,7 +2922,7 @@ megasas_complete_cmd(struct megasas_instance *instance,
struct megasas_cmd *cmd,
break;
default:
- dev_printk(KERN_DEBUG, &instance->pdev->dev, "MFI FW
status %#x\n",
+ dev_dbg(&instance->pdev->dev, "MFI FW status %#x\n",
hdr->cmd_status);
cmd->scmd->result = DID_ERROR << 16;
break;
@@ -3332,7 +3332,7 @@ megasas_transition_to_ready(struct megasas_instance
*instance, int ocr)
switch (fw_state) {
case MFI_STATE_F
[PATCH 2/3] megaraid_sas: Convert printk to printk_
Reduce object size a little by using pr_
calls instead of printk(KERN_.
Signed-off-by: Weidong Wang
---
drivers/scsi/megaraid/megaraid_sas_base.c | 14 +++---
1 file changed, 7 insertions(+), 7 deletions(-)
diff --git a/drivers/scsi/megaraid/megaraid_sas_base.c
b/drivers/scsi/megaraid/megaraid_sas_base.c
index ed9846d..2287aa1 100644
--- a/drivers/scsi/megaraid/megaraid_sas_base.c
+++ b/drivers/scsi/megaraid/megaraid_sas_base.c
@@ -5889,7 +5889,7 @@ static int megasas_mgmt_fasync(int fd, struct file
*filep, int mode)
return 0;
}
- printk(KERN_DEBUG "megasas: fasync_helper failed [%d]\n", rc);
+ pr_debug("megasas: fasync_helper failed [%d]\n", rc);
return rc;
}
@@ -6233,7 +6233,7 @@ static int megasas_mgmt_ioctl_aen(struct file *file,
unsigned long arg)
u32 wait_time = MEGASAS_RESET_WAIT_TIME;
if (file->private_data != file) {
- printk(KERN_DEBUG "megasas: fasync_helper was not "
+ pr_debug("megasas: fasync_helper was not "
"called first\n");
return -EINVAL;
}
@@ -6355,7 +6355,7 @@ static int megasas_mgmt_compat_ioctl_fw(struct file
*file, unsigned long arg)
if (copy_in_user(&cioc->frame.hdr.cmd_status,
&ioc->frame.hdr.cmd_status, sizeof(u8))) {
- printk(KERN_DEBUG "megasas: error copy_in_user cmd_status\n");
+ pr_debug("megasas: error copy_in_user cmd_status\n");
return -EFAULT;
}
return error;
@@ -6455,7 +6455,7 @@ megasas_sysfs_set_dbg_lvl(struct device_driver *dd, const
char *buf, size_t coun
int retval = count;
if (sscanf(buf, "%u", &megasas_dbg_lvl) < 1) {
- printk(KERN_ERR "megasas: could not set dbg_lvl\n");
+ pr_err("megasas: could not set dbg_lvl\n");
retval = -EINVAL;
}
return retval;
@@ -6480,7 +6480,7 @@ megasas_aen_polling(struct work_struct *work)
int error;
if (!instance) {
- printk(KERN_ERR "invalid instance!\n");
+ pr_err("invalid instance!\n");
kfree(ev);
return;
}
@@ -6740,7 +6740,7 @@ static int __init megasas_init(void)
rval = register_chrdev(0, "megaraid_sas_ioctl", &megasas_mgmt_fops);
if (rval < 0) {
- printk(KERN_DEBUG "megasas: failed to open device node\n");
+ pr_debug("megasas: failed to open device node\n");
return rval;
}
@@ -6752,7 +6752,7 @@ static int __init megasas_init(void)
rval = pci_register_driver(&megasas_pci_driver);
if (rval) {
- printk(KERN_DEBUG "megasas: PCI hotplug registration failed
\n");
+ pr_debug("megasas: PCI hotplug registration failed \n");
goto err_pcidrv;
}
--
1.9.0
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
[PATCH 0/3] megaraid_sas: copule of fixes
Fix some checkpatch Warnings. Fix a NULL pointer dereference. As well kernel >=3.0.y will have the 'NULL pointer dereference'. Weidong Wang (3): megaraid_sas: Convert dev_printk to dev_ megaraid_sas: Convert printk to printk_ megaraid_sas: return -ENOMEM when create DMA pool for cmd frames failed drivers/scsi/megaraid/megaraid_sas_base.c | 83 --- 1 file changed, 42 insertions(+), 41 deletions(-) -- 1.9.0 -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
[PATCH net-next v3] BNX2: fix a Null Pointer for stats_blk
we have two processes to do:
P1#: ifconfig eth0 down; which will call bnx2_close, then will
, and set Null to stats_blk
P2#: ifconfig eth0; which will call bnx2_get_stats64, it will
use stats_blk.
In one case:
--P1#-- --P2#--
stats_blk(no null)
bnx2_free_mem
->bp->stats_blk = NULL
GET_64BIT_NET_STATS
then it will cause 'NULL Pointer' Problem.
it is as well with 'ethtool -S ethx'.
Allocate the statistics block at probe time so that this problem is
impossible
Signed-off-by: Wang Weidong
---
Change in v2:
- Use Allocate the statistics block instead of spinlock, which
suggested by David Miller.
- Updating commit message according to changes.
Change in v3:
- bnx2_alloc_stats_blk is just allocating the stats block.
- use 'bp->status_blk' to store the status_blk which would used
in other funcs, just to key the codes simplified.
---
drivers/net/ethernet/broadcom/bnx2.c | 79
drivers/net/ethernet/broadcom/bnx2.h | 1 +
2 files changed, 53 insertions(+), 27 deletions(-)
diff --git a/drivers/net/ethernet/broadcom/bnx2.c
b/drivers/net/ethernet/broadcom/bnx2.c
index 2b66ef3..6259064 100644
--- a/drivers/net/ethernet/broadcom/bnx2.c
+++ b/drivers/net/ethernet/broadcom/bnx2.c
@@ -813,6 +813,46 @@ bnx2_alloc_rx_mem(struct bnx2 *bp)
}
static void
+bnx2_free_stats_blk(struct net_device *dev)
+{
+ struct bnx2 *bp = netdev_priv(dev);
+
+ if (bp->status_blk) {
+ dma_free_coherent(&bp->pdev->dev, bp->status_stats_size,
+ bp->status_blk,
+ bp->status_blk_mapping);
+ bp->status_blk = NULL;
+ bp->stats_blk = NULL;
+ }
+}
+
+static int
+bnx2_alloc_stats_blk(struct net_device *dev)
+{
+ int status_blk_size;
+ void *status_blk;
+ struct bnx2 *bp = netdev_priv(dev);
+
+ /* Combine status and statistics blocks into one allocation. */
+ status_blk_size = L1_CACHE_ALIGN(sizeof(struct status_block));
+ if (bp->flags & BNX2_FLAG_MSIX_CAP)
+ status_blk_size = L1_CACHE_ALIGN(BNX2_MAX_MSIX_HW_VEC *
+BNX2_SBLK_MSIX_ALIGN_SIZE);
+ bp->status_stats_size = status_blk_size +
+ sizeof(struct statistics_block);
+ status_blk = dma_zalloc_coherent(&bp->pdev->dev, bp->status_stats_size,
+&bp->status_blk_mapping, GFP_KERNEL);
+ if (status_blk == NULL)
+ return -ENOMEM;
+
+ bp->status_blk = status_blk;
+ bp->stats_blk = status_blk + status_blk_size;
+ bp->stats_blk_mapping = bp->status_blk_mapping + status_blk_size;
+
+ return 0;
+}
+
+static void
bnx2_free_mem(struct bnx2 *bp)
{
int i;
@@ -829,37 +869,19 @@ bnx2_free_mem(struct bnx2 *bp)
bp->ctx_blk[i] = NULL;
}
}
- if (bnapi->status_blk.msi) {
- dma_free_coherent(&bp->pdev->dev, bp->status_stats_size,
- bnapi->status_blk.msi,
- bp->status_blk_mapping);
+
+ if (bnapi->status_blk.msi)
bnapi->status_blk.msi = NULL;
- bp->stats_blk = NULL;
- }
}
static int
bnx2_alloc_mem(struct bnx2 *bp)
{
- int i, status_blk_size, err;
+ int i, err;
struct bnx2_napi *bnapi;
- void *status_blk;
-
- /* Combine status and statistics blocks into one allocation. */
- status_blk_size = L1_CACHE_ALIGN(sizeof(struct status_block));
- if (bp->flags & BNX2_FLAG_MSIX_CAP)
- status_blk_size = L1_CACHE_ALIGN(BNX2_MAX_MSIX_HW_VEC *
-BNX2_SBLK_MSIX_ALIGN_SIZE);
- bp->status_stats_size = status_blk_size +
- sizeof(struct statistics_block);
-
- status_blk = dma_zalloc_coherent(&bp->pdev->dev, bp->status_stats_size,
-&bp->status_blk_mapping, GFP_KERNEL);
- if (status_blk == NULL)
- goto alloc_mem_err;
bnapi = &bp->bnx2_napi[0];
- bnapi->status_blk.msi = status_blk;
+ bnapi->status_blk.msi = bp->status_blk;
bnapi->hw_tx_cons_ptr =
&bnapi->status_blk.msi->status_tx_quick_consumer_index0;
bnapi->hw_rx_cons_ptr =
@@ -870,7 +892,7 @@ bnx2_alloc_mem(struct bnx2 *bp)
bnapi = &bp->bnx2_napi[i];
- sblk = (status_blk + BNX2_SBLK_MSIX_ALIGN_SIZE * i);
+ sblk = (bp->status_blk + BNX2_SBLK_MSIX_ALIGN_SIZE * i);
bnapi->status_blk.msix = sblk;
bnapi->hw_tx_cons_ptr =
&sblk->status_tx_quick_consumer_index;
@@ -880,10 +902,6 @@ bnx2_alloc_mem(struct bnx2 *bp)
}
[PATCH net-next v2 RESEND] BNX2: fix a Null Pointer for stats_blk
we have two processes to do:
P1#: ifconfig eth0 down; which will call bnx2_close, then will
, and set Null to stats_blk
P2#: ifconfig eth0; which will call bnx2_get_stats64, it will
use stats_blk.
In one case:
--P1#-- --P2#--
stats_blk(no null)
bnx2_free_mem
->bp->stats_blk = NULL
GET_64BIT_NET_STATS
then it will cause 'NULL Pointer' Problem.
it is as well with 'ethtool -S ethx'.
Allocate the statistics block at probe time so that this problem is
impossible
Signed-off-by: Weidong Wang
---
Change in v2:
- Use Allocate the statistics block instead of spinlock, which
suggested by David Miller.
- Updating commit message according to changes.
---
drivers/net/ethernet/broadcom/bnx2.c | 61 +++-
1 file changed, 40 insertions(+), 21 deletions(-)
diff --git a/drivers/net/ethernet/broadcom/bnx2.c
b/drivers/net/ethernet/broadcom/bnx2.c
index 2b66ef3..1f33982 100644
--- a/drivers/net/ethernet/broadcom/bnx2.c
+++ b/drivers/net/ethernet/broadcom/bnx2.c
@@ -813,22 +813,11 @@ bnx2_alloc_rx_mem(struct bnx2 *bp)
}
static void
-bnx2_free_mem(struct bnx2 *bp)
+bnx2_free_stats_blk(struct net_device *dev)
{
- int i;
+ struct bnx2 *bp = netdev_priv(dev);
struct bnx2_napi *bnapi = &bp->bnx2_napi[0];
- bnx2_free_tx_mem(bp);
- bnx2_free_rx_mem(bp);
-
- for (i = 0; i < bp->ctx_pages; i++) {
- if (bp->ctx_blk[i]) {
- dma_free_coherent(&bp->pdev->dev, BNX2_PAGE_SIZE,
- bp->ctx_blk[i],
- bp->ctx_blk_mapping[i]);
- bp->ctx_blk[i] = NULL;
- }
- }
if (bnapi->status_blk.msi) {
dma_free_coherent(&bp->pdev->dev, bp->status_stats_size,
bnapi->status_blk.msi,
@@ -839,11 +828,12 @@ bnx2_free_mem(struct bnx2 *bp)
}
static int
-bnx2_alloc_mem(struct bnx2 *bp)
+bnx2_alloc_stats_blk(struct net_device *dev)
{
- int i, status_blk_size, err;
+ int i, status_blk_size;
struct bnx2_napi *bnapi;
void *status_blk;
+ struct bnx2 *bp = netdev_priv(dev);
/* Combine status and statistics blocks into one allocation. */
status_blk_size = L1_CACHE_ALIGN(sizeof(struct status_block));
@@ -852,11 +842,10 @@ bnx2_alloc_mem(struct bnx2 *bp)
BNX2_SBLK_MSIX_ALIGN_SIZE);
bp->status_stats_size = status_blk_size +
sizeof(struct statistics_block);
-
status_blk = dma_zalloc_coherent(&bp->pdev->dev, bp->status_stats_size,
&bp->status_blk_mapping, GFP_KERNEL);
if (status_blk == NULL)
- goto alloc_mem_err;
+ return -ENOMEM;
bnapi = &bp->bnx2_napi[0];
bnapi->status_blk.msi = status_blk;
@@ -865,11 +854,10 @@ bnx2_alloc_mem(struct bnx2 *bp)
bnapi->hw_rx_cons_ptr =
&bnapi->status_blk.msi->status_rx_quick_consumer_index0;
if (bp->flags & BNX2_FLAG_MSIX_CAP) {
- for (i = 1; i < bp->irq_nvecs; i++) {
+ for (i = 1; i < BNX2_MAX_MSIX_HW_VEC; i++) {
struct status_block_msix *sblk;
bnapi = &bp->bnx2_napi[i];
-
sblk = (status_blk + BNX2_SBLK_MSIX_ALIGN_SIZE * i);
bnapi->status_blk.msix = sblk;
bnapi->hw_tx_cons_ptr =
@@ -879,11 +867,35 @@ bnx2_alloc_mem(struct bnx2 *bp)
bnapi->int_num = i << 24;
}
}
-
bp->stats_blk = status_blk + status_blk_size;
-
bp->stats_blk_mapping = bp->status_blk_mapping + status_blk_size;
+ return 0;
+}
+
+static void
+bnx2_free_mem(struct bnx2 *bp)
+{
+ int i;
+
+ bnx2_free_tx_mem(bp);
+ bnx2_free_rx_mem(bp);
+
+ for (i = 0; i < bp->ctx_pages; i++) {
+ if (bp->ctx_blk[i]) {
+ dma_free_coherent(&bp->pdev->dev, BNX2_PAGE_SIZE,
+ bp->ctx_blk[i],
+ bp->ctx_blk_mapping[i]);
+ bp->ctx_blk[i] = NULL;
+ }
+ }
+}
+
+static int
+bnx2_alloc_mem(struct bnx2 *bp)
+{
+ int i, err;
+
if (BNX2_CHIP(bp) == BNX2_CHIP_5709) {
bp->ctx_pages = 0x2000 / BNX2_PAGE_SIZE;
if (bp->ctx_pages == 0)
@@ -8330,6 +8342,11 @@ bnx2_init_board(struct pci_dev *pdev, struct net_device
*dev)
bp->phy_addr = 1;
+ /* allocate stats_blk */
+ rc = bnx2_alloc_stats_blk(dev
Re: [PATCH net-next v2] BNX2: fix a Null Pointer for stats_blk
On 2015/9/28 15:01, Weidong Wang wrote: > we have two processes to do: > P1#: ifconfig eth0 down; which will call bnx2_close, then will > , and set Null to stats_blk > P2#: ifconfig eth0; which will call bnx2_get_stats64, it will > use stats_blk. > In one case: > --P1#-- --P2#-- > stats_blk(no null) > bnx2_free_mem > ->bp->stats_blk = NULL > GET_64BIT_NET_STATS > > then it will cause 'NULL Pointer' Problem. > it is as well with 'ethtool -S ethx'. > > Allocate the statistics block at probe time so that this problem is > impossible > > Signed-off-by: Tianhong Ding > --- Sorry for that, The sob is Error. I will fixed it. Just Ignore it. Regards. Weidong -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
[PATCH net-next v2] BNX2: fix a Null Pointer for stats_blk
we have two processes to do:
P1#: ifconfig eth0 down; which will call bnx2_close, then will
, and set Null to stats_blk
P2#: ifconfig eth0; which will call bnx2_get_stats64, it will
use stats_blk.
In one case:
--P1#-- --P2#--
stats_blk(no null)
bnx2_free_mem
->bp->stats_blk = NULL
GET_64BIT_NET_STATS
then it will cause 'NULL Pointer' Problem.
it is as well with 'ethtool -S ethx'.
Allocate the statistics block at probe time so that this problem is
impossible
Signed-off-by: Tianhong Ding
---
Change in v2:
- Use Allocate the statistics block instead of spinlock, which
suggested by David Miller.
- Updating commit message according to changes.
---
drivers/net/ethernet/broadcom/bnx2.c | 61 +++-
1 file changed, 40 insertions(+), 21 deletions(-)
diff --git a/drivers/net/ethernet/broadcom/bnx2.c
b/drivers/net/ethernet/broadcom/bnx2.c
index 2b66ef3..1f33982 100644
--- a/drivers/net/ethernet/broadcom/bnx2.c
+++ b/drivers/net/ethernet/broadcom/bnx2.c
@@ -813,22 +813,11 @@ bnx2_alloc_rx_mem(struct bnx2 *bp)
}
static void
-bnx2_free_mem(struct bnx2 *bp)
+bnx2_free_stats_blk(struct net_device *dev)
{
- int i;
+ struct bnx2 *bp = netdev_priv(dev);
struct bnx2_napi *bnapi = &bp->bnx2_napi[0];
- bnx2_free_tx_mem(bp);
- bnx2_free_rx_mem(bp);
-
- for (i = 0; i < bp->ctx_pages; i++) {
- if (bp->ctx_blk[i]) {
- dma_free_coherent(&bp->pdev->dev, BNX2_PAGE_SIZE,
- bp->ctx_blk[i],
- bp->ctx_blk_mapping[i]);
- bp->ctx_blk[i] = NULL;
- }
- }
if (bnapi->status_blk.msi) {
dma_free_coherent(&bp->pdev->dev, bp->status_stats_size,
bnapi->status_blk.msi,
@@ -839,11 +828,12 @@ bnx2_free_mem(struct bnx2 *bp)
}
static int
-bnx2_alloc_mem(struct bnx2 *bp)
+bnx2_alloc_stats_blk(struct net_device *dev)
{
- int i, status_blk_size, err;
+ int i, status_blk_size;
struct bnx2_napi *bnapi;
void *status_blk;
+ struct bnx2 *bp = netdev_priv(dev);
/* Combine status and statistics blocks into one allocation. */
status_blk_size = L1_CACHE_ALIGN(sizeof(struct status_block));
@@ -852,11 +842,10 @@ bnx2_alloc_mem(struct bnx2 *bp)
BNX2_SBLK_MSIX_ALIGN_SIZE);
bp->status_stats_size = status_blk_size +
sizeof(struct statistics_block);
-
status_blk = dma_zalloc_coherent(&bp->pdev->dev, bp->status_stats_size,
&bp->status_blk_mapping, GFP_KERNEL);
if (status_blk == NULL)
- goto alloc_mem_err;
+ return -ENOMEM;
bnapi = &bp->bnx2_napi[0];
bnapi->status_blk.msi = status_blk;
@@ -865,11 +854,10 @@ bnx2_alloc_mem(struct bnx2 *bp)
bnapi->hw_rx_cons_ptr =
&bnapi->status_blk.msi->status_rx_quick_consumer_index0;
if (bp->flags & BNX2_FLAG_MSIX_CAP) {
- for (i = 1; i < bp->irq_nvecs; i++) {
+ for (i = 1; i < BNX2_MAX_MSIX_HW_VEC; i++) {
struct status_block_msix *sblk;
bnapi = &bp->bnx2_napi[i];
-
sblk = (status_blk + BNX2_SBLK_MSIX_ALIGN_SIZE * i);
bnapi->status_blk.msix = sblk;
bnapi->hw_tx_cons_ptr =
@@ -879,11 +867,35 @@ bnx2_alloc_mem(struct bnx2 *bp)
bnapi->int_num = i << 24;
}
}
-
bp->stats_blk = status_blk + status_blk_size;
-
bp->stats_blk_mapping = bp->status_blk_mapping + status_blk_size;
+ return 0;
+}
+
+static void
+bnx2_free_mem(struct bnx2 *bp)
+{
+ int i;
+
+ bnx2_free_tx_mem(bp);
+ bnx2_free_rx_mem(bp);
+
+ for (i = 0; i < bp->ctx_pages; i++) {
+ if (bp->ctx_blk[i]) {
+ dma_free_coherent(&bp->pdev->dev, BNX2_PAGE_SIZE,
+ bp->ctx_blk[i],
+ bp->ctx_blk_mapping[i]);
+ bp->ctx_blk[i] = NULL;
+ }
+ }
+}
+
+static int
+bnx2_alloc_mem(struct bnx2 *bp)
+{
+ int i, err;
+
if (BNX2_CHIP(bp) == BNX2_CHIP_5709) {
bp->ctx_pages = 0x2000 / BNX2_PAGE_SIZE;
if (bp->ctx_pages == 0)
@@ -8330,6 +8342,11 @@ bnx2_init_board(struct pci_dev *pdev, struct net_device
*dev)
bp->phy_addr = 1;
+ /* allocate stats_blk */
+ rc = bnx2_alloc_stats_blk(dev);
+ if (rc)
+ goto err_out_unmap;
+
/* Disable WOL support if we are running on a SERDES chip. */
if (BNX2_CHIP(bp) == BNX2_CHIP_5709)
bnx2_get_5709_media(bp);
@@
Re: [PATCH net-next] BNX2: fix a Null Pointer for stats_blk
On 2015/9/24 13:34, David Miller wrote: > From: Weidong Wang > Date: Thu, 24 Sep 2015 10:00:45 +0800 > >> It does affect the intention. Although, the problem exists then makes the >> system panic within some case. >> >> Do you have any idea about it? > > Allocate the statistics block at probe time so that this problem is > impossible. > It is a good idea. Yet, what is the intention of the dynamic to alloc/free stats_block? what will be affected by allocating the statistics block. Best Regards, Weidong > -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH net-next] BNX2: fix a Null Pointer for stats_blk
On 2015/9/24 6:31, David Miller wrote: > From: Weidong Wang > Date: Tue, 22 Sep 2015 20:42:40 +0800 > >> @@ -880,6 +882,7 @@ bnx2_alloc_mem(struct bnx2 *bp) >> } >> } >> >> +spin_lock(&bp->stats64_lock); >> bp->stats_blk = status_blk + status_blk_size; >> >> bp->stats_blk_mapping = bp->status_blk_mapping + status_blk_size; >> @@ -894,20 +897,23 @@ bnx2_alloc_mem(struct bnx2 *bp) >> &bp->ctx_blk_mapping[i], >> GFP_KERNEL); >> if (bp->ctx_blk[i] == NULL) >> -goto alloc_mem_err; >> +goto free_stats64_lock; >> } >> } >> >> err = bnx2_alloc_rx_mem(bp); >> if (err) >> -goto alloc_mem_err; >> +goto free_stats64_lock; > > You're holding a spinlock while doing GFP_KERNEL allocations. > hm, yep, I should move it after the allocations. Like this: @@ -880,7 +882,9 @@ bnx2_alloc_mem(struct bnx2 *bp) } } + spin_lock(&bp->stats64_lock); bp->stats_blk = status_blk + status_blk_size; + spin_unlock(&bp->stats64_lock); the allocations won't use the stats_blk, so I shouldn't hold the lock while doing allocations. > Second of all, taking a spinlock in get_stats64() defeats the whole > intention of making statistics acquisition as fast and as SMP scalable > as possible. > It does affect the intention. Although, the problem exists then makes the system panic within some case. Do you have any idea about it? Best Regards, Weidong > . > -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
[PATCH net-next] BNX2: fix a Null Pointer for stats_blk
we have two processes to do:
P1#: ifconfig eth0 down; which will call bnx2_close, then will
, and set Null to stats_blk
P2#: ifconfig eth0; which will call bnx2_get_stats64, it will
use stats_blk.
In one case:
--P1#-- --P2#--
stats_blk(no null)
bnx2_free_mem
->bp->stats_blk = NULL
GET_64BIT_NET_STATS
then it will cause 'NULL Pointer' Problem.
it is as well with 'ethtool -S ethx'.
BTW, the other branch has this problem as well.
So we add a spin_lock to protect stats_blk.
Signed-off-by: Wang Weidong
---
drivers/net/ethernet/broadcom/bnx2.c | 42
drivers/net/ethernet/broadcom/bnx2.h | 1 +
2 files changed, 34 insertions(+), 9 deletions(-)
diff --git a/drivers/net/ethernet/broadcom/bnx2.c
b/drivers/net/ethernet/broadcom/bnx2.c
index 2b66ef3..aec4081 100644
--- a/drivers/net/ethernet/broadcom/bnx2.c
+++ b/drivers/net/ethernet/broadcom/bnx2.c
@@ -830,11 +830,13 @@ bnx2_free_mem(struct bnx2 *bp)
}
}
if (bnapi->status_blk.msi) {
+ spin_lock(&bp->stats64_lock);
dma_free_coherent(&bp->pdev->dev, bp->status_stats_size,
bnapi->status_blk.msi,
bp->status_blk_mapping);
bnapi->status_blk.msi = NULL;
bp->stats_blk = NULL;
+ spin_unlock(&bp->stats64_lock);
}
}
@@ -880,6 +882,7 @@ bnx2_alloc_mem(struct bnx2 *bp)
}
}
+ spin_lock(&bp->stats64_lock);
bp->stats_blk = status_blk + status_blk_size;
bp->stats_blk_mapping = bp->status_blk_mapping + status_blk_size;
@@ -894,20 +897,23 @@ bnx2_alloc_mem(struct bnx2 *bp)
&bp->ctx_blk_mapping[i],
GFP_KERNEL);
if (bp->ctx_blk[i] == NULL)
- goto alloc_mem_err;
+ goto free_stats64_lock;
}
}
err = bnx2_alloc_rx_mem(bp);
if (err)
- goto alloc_mem_err;
+ goto free_stats64_lock;
err = bnx2_alloc_tx_mem(bp);
if (err)
- goto alloc_mem_err;
+ goto free_stats64_lock;
+ spin_unlock(&bp->stats64_lock);
return 0;
+free_stats64_lock:
+ spin_unlock(&bp->stats64_lock);
alloc_mem_err:
bnx2_free_mem(bp);
return -ENOMEM;
@@ -6756,10 +6762,14 @@ bnx2_close(struct net_device *dev)
static void
bnx2_save_stats(struct bnx2 *bp)
{
- u32 *hw_stats = (u32 *) bp->stats_blk;
- u32 *temp_stats = (u32 *) bp->temp_stats_blk;
+ u32 *hw_stats;
+ u32 *temp_stats;
int i;
+ spin_lock(&bp->stats64_lock);
+ hw_stats = (u32 *) bp->stats_blk;
+ temp_stats = (u32 *) bp->temp_stats_blk;
+
/* The 1st 10 counters are 64-bit counters */
for (i = 0; i < 20; i += 2) {
u32 hi;
@@ -6775,6 +6785,8 @@ bnx2_save_stats(struct bnx2 *bp)
for ( ; i < sizeof(struct statistics_block) / 4; i++)
temp_stats[i] += hw_stats[i];
+
+ spin_unlock(&bp->stats64_lock);
}
#define GET_64BIT_NET_STATS64(ctr) \
@@ -6793,8 +6805,11 @@ bnx2_get_stats64(struct net_device *dev, struct
rtnl_link_stats64 *net_stats)
{
struct bnx2 *bp = netdev_priv(dev);
- if (bp->stats_blk == NULL)
+ spin_lock(&bp->stats64_lock);
+ if (bp->stats_blk == NULL) {
+ spin_unlock(&bp->stats64_lock);
return net_stats;
+ }
net_stats->rx_packets =
GET_64BIT_NET_STATS(stat_IfHCInUcastPkts) +
@@ -6858,6 +6873,7 @@ bnx2_get_stats64(struct net_device *dev, struct
rtnl_link_stats64 *net_stats)
GET_32BIT_NET_STATS(stat_IfInMBUFDiscards) +
GET_32BIT_NET_STATS(stat_FwRxDrop);
+ spin_unlock(&bp->stats64_lock);
return net_stats;
}
@@ -7634,13 +7650,17 @@ bnx2_get_ethtool_stats(struct net_device *dev,
{
struct bnx2 *bp = netdev_priv(dev);
int i;
- u32 *hw_stats = (u32 *) bp->stats_blk;
- u32 *temp_stats = (u32 *) bp->temp_stats_blk;
+ u32 *hw_stats;
+ u32 *temp_stats;
u8 *stats_len_arr = NULL;
+ spin_lock(&bp->stats64_lock);
+ hw_stats = (u32 *) bp->stats_blk;
+ temp_stats = (u32 *) bp->temp_stats_blk;
+
if (hw_stats == NULL) {
memset(buf, 0, sizeof(u64) * BNX2_NUM_STATS);
- return;
+ goto free_stats64_lock;
}
if ((BNX2_CHIP_ID(bp) == BNX2_CHIP_ID_5706_A0) ||
@@ -7673,6 +7693,9 @@ bnx2_get_ethtool_stats(struct net_device *dev,
(((u64) *(temp_stats + offset)) << 32) +
*(temp_stats + offset + 1);
}
+
+free_stats64_lock:
+ spin_unlock(&bp->stats64_loc
