Re: [PATCH v2 2/6] staging: erofs: code cleanup for option parsing of fault_injection
Hi Chengguang,
On 2018/9/17 23:34, Chengguang Xu wrote:
> Define a dummpy function of erofs_build_fault_attr() when macro
> CONFIG_EROFS_FAULT_INJECTION is disabled, so that we don't have to
> check the macro in calling place. Based on above adjustment,
> do proper code cleanup for option parsing of fault_injection.
>
> Signed-off-by: Chengguang Xu
> ---
> drivers/staging/erofs/super.c | 33 -
> 1 file changed, 20 insertions(+), 13 deletions(-)
>
> diff --git a/drivers/staging/erofs/super.c b/drivers/staging/erofs/super.c
> index 9e421536cbdf..7ce2fd3d49f3 100644
> --- a/drivers/staging/erofs/super.c
> +++ b/drivers/staging/erofs/super.c
> @@ -145,10 +145,14 @@ char *erofs_fault_name[FAULT_MAX] = {
> [FAULT_KMALLOC] = "kmalloc",
> };
>
> -static void erofs_build_fault_attr(struct erofs_sb_info *sbi,
> - unsigned int rate)
> +static int erofs_build_fault_attr(struct erofs_sb_info *sbi,
> + substring_t *args)
> {
> struct erofs_fault_info *ffi = &sbi->fault_info;
> + int rate = 0;
> +
> + if (args->from && match_int(args, &rate))
> + return -EINVAL;
>
> if (rate) {
> atomic_set(&ffi->inject_ops, 0);
> @@ -157,6 +161,15 @@ static void erofs_build_fault_attr(struct erofs_sb_info
> *sbi,
> } else {
> memset(ffi, 0, sizeof(struct erofs_fault_info));
> }
> +
> + set_opt(sbi, FAILt_INJECTION);
drivers/staging/erofs/super.c: In function ‘__erofs_build_fault_attr’:
drivers/staging/erofs/internal.h:176:51: error: ‘EROFS_MOUNT_FAILt_INJECTION’
undeclared (first use in this function)
#define set_opt(sbi, option) ((sbi)->mount_opt |= EROFS_MOUNT_##option)
^
drivers/staging/erofs/super.c:166:2: note: in expansion of macro ‘set_opt’
set_opt(sbi, FAILt_INJECTION);
^
drivers/staging/erofs/internal.h:176:51: note: each undeclared identifier is
reported only once for each function it appears in
#define set_opt(sbi, option) ((sbi)->mount_opt |= EROFS_MOUNT_##option)
^
drivers/staging/erofs/super.c:166:2: note: in expansion of macro ‘set_opt’
set_opt(sbi, FAILt_INJECTION);
^
Thanks,
Gao Xiang
> +}
> +#else
> +static int erofs_build_fault_attr(struct erofs_sb_info *sbi,
> + substring_t *args)
> +{
> + infoln("fault_injection options not supported");
> + return 0;
> }
> #endif
>
> @@ -193,7 +206,7 @@ static int parse_options(struct super_block *sb, char
> *options)
> {
> substring_t args[MAX_OPT_ARGS];
> char *p;
> - int arg = 0;
> + int err;
>
> if (!options)
> return 0;
> @@ -238,18 +251,12 @@ static int parse_options(struct super_block *sb, char
> *options)
> infoln("noacl options not supported");
> break;
> #endif
> -#ifdef CONFIG_EROFS_FAULT_INJECTION
> - case Opt_fault_injection:
> - if (args->from && match_int(args, &arg))
> - return -EINVAL;
> - erofs_build_fault_attr(EROFS_SB(sb), arg);
> - set_opt(EROFS_SB(sb), FAULT_INJECTION);
> - break;
> -#else
> case Opt_fault_injection:
> - infoln("fault_injection options not supported");
> + err = erofs_build_fault_attr(EROFS_SB(sb), args);
> + if (err)
> + return err;
> break;
> -#endif
> +
> default:
> errln("Unrecognized mount option \"%s\" "
> "or missing value", p);
>
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [patch 09/11] x86/vdso: Simplify the invalid vclock case
On Mon, 17 Sep 2018, John Stultz wrote: > On Mon, Sep 17, 2018 at 12:25 PM, Andy Lutomirski wrote: > > Also, I'm not entirely convinced that this "last" thing is needed at > > all. John, what's the scenario under which we need it? > > So my memory is probably a bit foggy, but I recall that as we > accelerated gettimeofday, we found that even on systems that claimed > to have synced TSCs, they were actually just slightly out of sync. > Enough that right after cycles_last had been updated, a read on > another cpu could come in just behind cycles_last, resulting in a > negative interval causing lots of havoc. > > So the sanity check is needed to avoid that case. Your memory serves you right. That's indeed observable on CPUs which lack TSC_ADJUST. @Andy: Welcome to the wonderful world of TSC. Thanks, tglx ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH v2 1/8] staging: rtl8188eu: simplify calculation
Simplify calcualation: * 10 / 2 can be reduced to * 5 Also cleans missing spaces checkpatch issues. Signed-off-by: Michael Straube --- changes in v2: fixed typo in patch 1/8 commit message Simpliy -> Simplify drivers/staging/rtl8188eu/core/rtw_ioctl_set.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/staging/rtl8188eu/core/rtw_ioctl_set.c b/drivers/staging/rtl8188eu/core/rtw_ioctl_set.c index c040f185074b..0880f18520a0 100644 --- a/drivers/staging/rtl8188eu/core/rtw_ioctl_set.c +++ b/drivers/staging/rtl8188eu/core/rtw_ioctl_set.c @@ -575,7 +575,7 @@ u16 rtw_get_cur_max_rate(struct adapter *adapter) i++; } - max_rate = max_rate*10/2; + max_rate *= 5; } return max_rate; -- 2.19.0 ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH v2 7/8] staging: rtl8188eu: fix lines over 80 characters
Wrap lines over 80 characters where appropriate to
clear checkpatch warnings.
Signed-off-by: Michael Straube
---
drivers/staging/rtl8188eu/core/rtw_ioctl_set.c | 15 ++-
1 file changed, 10 insertions(+), 5 deletions(-)
diff --git a/drivers/staging/rtl8188eu/core/rtw_ioctl_set.c
b/drivers/staging/rtl8188eu/core/rtw_ioctl_set.c
index d6d6a232c4ec..c291d3a0fdcc 100644
--- a/drivers/staging/rtl8188eu/core/rtw_ioctl_set.c
+++ b/drivers/staging/rtl8188eu/core/rtw_ioctl_set.c
@@ -339,7 +339,8 @@ u8 rtw_set_802_11_infrastructure_mode(struct adapter
*padapter,
check_fwstate(pmlmepriv, WIFI_ADHOC_MASTER_STATE))
rtw_free_assoc_resources(padapter);
- if (*pold_state == Ndis802_11Infrastructure || *pold_state ==
Ndis802_11IBSS) {
+ if (*pold_state == Ndis802_11Infrastructure ||
+ *pold_state == Ndis802_11IBSS) {
if (check_fwstate(pmlmepriv, _FW_LINKED))
rtw_indicate_disconnect(padapter); /* will clr
Linked_state; before this function, we must have checked whether issue
dis-assoc_cmd or not */
}
@@ -444,7 +445,8 @@ u8 rtw_set_802_11_authentication_mode(struct adapter
*padapter, enum ndis_802_11
int res;
u8 ret;
- RT_TRACE(_module_rtl871x_ioctl_set_c_, _drv_info_,
("set_802_11_auth.mode(): mode =%x\n", authmode));
+ RT_TRACE(_module_rtl871x_ioctl_set_c_, _drv_info_,
+("set_802_11_auth.mode(): mode =%x\n", authmode));
psecuritypriv->ndisauthtype = authmode;
@@ -497,7 +499,8 @@ u8 rtw_set_802_11_add_wep(struct adapter *padapter, struct
ndis_802_11_wep *wep)
("rtw_set_802_11_add_wep:before memcpy, wep->KeyLength = 0x%x
wep->KeyIndex = 0x%x keyid =%x\n",
wep->KeyLength, wep->KeyIndex, keyid));
- memcpy(&psecuritypriv->dot11DefKey[keyid].skey[0], &wep->KeyMaterial,
wep->KeyLength);
+ memcpy(&psecuritypriv->dot11DefKey[keyid].skey[0],
+ &wep->KeyMaterial, wep->KeyLength);
psecuritypriv->dot11DefKeylen[keyid] = wep->KeyLength;
@@ -551,7 +554,8 @@ u16 rtw_get_cur_max_rate(struct adapter *adapter)
return 0;
if (pmlmeext->cur_wireless_mode & (WIRELESS_11_24N | WIRELESS_11_5N)) {
- p = rtw_get_ie(&pcur_bss->ies[12], _HT_CAPABILITY_IE_,
&ht_ielen, pcur_bss->ie_length - 12);
+ p = rtw_get_ie(&pcur_bss->ies[12], _HT_CAPABILITY_IE_,
+ &ht_ielen, pcur_bss->ie_length - 12);
if (p && ht_ielen > 0) {
/* cur_bwmod is updated by beacon, pmlmeinfo is updated
by association response */
bw_40MHz = (pmlmeext->cur_bwmode &&
(HT_INFO_HT_PARAM_REC_TRANS_CHNL_WIDTH & pmlmeinfo->HT_info.infos[0])) ? 1 : 0;
@@ -568,7 +572,8 @@ u16 rtw_get_cur_max_rate(struct adapter *adapter)
);
}
} else {
- while (pcur_bss->SupportedRates[i] != 0 &&
pcur_bss->SupportedRates[i] != 0xFF) {
+ while (pcur_bss->SupportedRates[i] != 0 &&
+ pcur_bss->SupportedRates[i] != 0xFF) {
rate = pcur_bss->SupportedRates[i] & 0x7F;
if (rate > max_rate)
max_rate = rate;
--
2.19.0
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH v2 4/8] staging: rtl8188eu: fix comparsions to true
Use if(x) instead of if(x == true).
Signed-off-by: Michael Straube
---
.../staging/rtl8188eu/core/rtw_ioctl_set.c| 26 +--
1 file changed, 13 insertions(+), 13 deletions(-)
diff --git a/drivers/staging/rtl8188eu/core/rtw_ioctl_set.c
b/drivers/staging/rtl8188eu/core/rtw_ioctl_set.c
index a00272540846..bd82de044bba 100644
--- a/drivers/staging/rtl8188eu/core/rtw_ioctl_set.c
+++ b/drivers/staging/rtl8188eu/core/rtw_ioctl_set.c
@@ -67,7 +67,7 @@ u8 rtw_do_join(struct adapter *padapter)
mod_timer(&pmlmepriv->assoc_timer,
jiffies + msecs_to_jiffies(MAX_JOIN_TIMEOUT));
} else {
- if (check_fwstate(pmlmepriv, WIFI_ADHOC_STATE) == true)
{
+ if (check_fwstate(pmlmepriv, WIFI_ADHOC_STATE)) {
/* submit createbss_cmd to change to a
ADHOC_MASTER */
/* pmlmepriv->lock has been acquired by
caller... */
@@ -136,7 +136,7 @@ u8 rtw_set_802_11_bssid(struct adapter *padapter, u8 *bssid)
spin_lock_bh(&pmlmepriv->lock);
DBG_88E("Set BSSID under fw_state = 0x%08x\n", get_fwstate(pmlmepriv));
- if (check_fwstate(pmlmepriv, _FW_UNDER_SURVEY) == true)
+ if (check_fwstate(pmlmepriv, _FW_UNDER_SURVEY))
goto handle_tkip_countermeasure;
else if (check_fwstate(pmlmepriv, _FW_UNDER_LINKING))
goto release_mlme_lock;
@@ -154,12 +154,12 @@ u8 rtw_set_802_11_bssid(struct adapter *padapter, u8
*bssid)
rtw_disassoc_cmd(padapter, 0, true);
- if (check_fwstate(pmlmepriv, _FW_LINKED) == true)
+ if (check_fwstate(pmlmepriv, _FW_LINKED))
rtw_indicate_disconnect(padapter);
rtw_free_assoc_resources(padapter);
- if ((check_fwstate(pmlmepriv, WIFI_ADHOC_MASTER_STATE)
== true)) {
+ if (check_fwstate(pmlmepriv, WIFI_ADHOC_MASTER_STATE)) {
_clr_fwstate_(pmlmepriv,
WIFI_ADHOC_MASTER_STATE);
set_fwstate(pmlmepriv, WIFI_ADHOC_STATE);
}
@@ -220,9 +220,9 @@ u8 rtw_set_802_11_ssid(struct adapter *padapter, struct
ndis_802_11_ssid *ssid)
spin_lock_bh(&pmlmepriv->lock);
DBG_88E("Set SSID under fw_state = 0x%08x\n", get_fwstate(pmlmepriv));
- if (check_fwstate(pmlmepriv, _FW_UNDER_SURVEY) == true)
+ if (check_fwstate(pmlmepriv, _FW_UNDER_SURVEY))
goto handle_tkip_countermeasure;
- else if (check_fwstate(pmlmepriv, _FW_UNDER_LINKING) == true)
+ else if (check_fwstate(pmlmepriv, _FW_UNDER_LINKING))
goto release_mlme_lock;
if (check_fwstate(pmlmepriv, _FW_LINKED|WIFI_ADHOC_MASTER_STATE)) {
@@ -240,12 +240,12 @@ u8 rtw_set_802_11_ssid(struct adapter *padapter, struct
ndis_802_11_ssid *ssid)
/* if in WIFI_ADHOC_MASTER_STATE |
WIFI_ADHOC_STATE, create bss or rejoin again */
rtw_disassoc_cmd(padapter, 0, true);
- if (check_fwstate(pmlmepriv,
_FW_LINKED) == true)
+ if (check_fwstate(pmlmepriv,
_FW_LINKED))
rtw_indicate_disconnect(padapter);
rtw_free_assoc_resources(padapter);
- if (check_fwstate(pmlmepriv,
WIFI_ADHOC_MASTER_STATE) == true) {
+ if (check_fwstate(pmlmepriv,
WIFI_ADHOC_MASTER_STATE)) {
_clr_fwstate_(pmlmepriv,
WIFI_ADHOC_MASTER_STATE);
set_fwstate(pmlmepriv,
WIFI_ADHOC_STATE);
}
@@ -262,12 +262,12 @@ u8 rtw_set_802_11_ssid(struct adapter *padapter, struct
ndis_802_11_ssid *ssid)
rtw_disassoc_cmd(padapter, 0, true);
- if (check_fwstate(pmlmepriv, _FW_LINKED) == true)
+ if (check_fwstate(pmlmepriv, _FW_LINKED))
rtw_indicate_disconnect(padapter);
rtw_free_assoc_resources(padapter);
- if (check_fwstate(pmlmepriv, WIFI_ADHOC_MASTER_STATE)
== true) {
+ if (check_fwstate(pmlmepriv, WIFI_ADHOC_MASTER_STATE)) {
_clr_fwstate_(pmlmepriv,
WIFI_ADHOC_MASTER_STATE);
set_fwstate(pmlmepriv, WIFI_ADHOC_STATE);
}
@@ -291,7 +291,7 @@ u8 rtw_set_802_11_ssid(struct adapter *padapter, struct
ndis_802_11_ssid *ssid)
memcpy(&pmlmepriv->assoc_ssid, ssid, sizeof(struct ndis_802_11_ssid));
pmlmepriv->assoc_by_bssid = false
[PATCH v2 5/8] staging: rtl8188eu: fix comparsions to false
Use if(!x) instead of if(x == false).
Signed-off-by: Michael Straube
---
drivers/staging/rtl8188eu/core/rtw_ioctl_set.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/drivers/staging/rtl8188eu/core/rtw_ioctl_set.c
b/drivers/staging/rtl8188eu/core/rtw_ioctl_set.c
index bd82de044bba..db983f15ddd6 100644
--- a/drivers/staging/rtl8188eu/core/rtw_ioctl_set.c
+++ b/drivers/staging/rtl8188eu/core/rtw_ioctl_set.c
@@ -145,7 +145,7 @@ u8 rtw_set_802_11_bssid(struct adapter *padapter, u8 *bssid)
RT_TRACE(_module_rtl871x_ioctl_set_c_, _drv_info_, ("set_bssid:
_FW_LINKED||WIFI_ADHOC_MASTER_STATE\n"));
if (!memcmp(&pmlmepriv->cur_network.network.MacAddress, bssid,
ETH_ALEN)) {
- if (check_fwstate(pmlmepriv, WIFI_STATION_STATE) ==
false)
+ if (!check_fwstate(pmlmepriv, WIFI_STATION_STATE))
goto release_mlme_lock;/* it means driver is in
WIFI_ADHOC_MASTER_STATE, we needn't create bss again. */
} else {
RT_TRACE(_module_rtl871x_ioctl_set_c_, _drv_info_,
("Set BSSID not the same bssid\n"));
@@ -231,7 +231,7 @@ u8 rtw_set_802_11_ssid(struct adapter *padapter, struct
ndis_802_11_ssid *ssid)
if (pmlmepriv->assoc_ssid.SsidLength == ssid->SsidLength &&
!memcmp(&pmlmepriv->assoc_ssid.Ssid, ssid->Ssid,
ssid->SsidLength)) {
- if ((check_fwstate(pmlmepriv, WIFI_STATION_STATE) ==
false)) {
+ if (!check_fwstate(pmlmepriv, WIFI_STATION_STATE)) {
RT_TRACE(_module_rtl871x_ioctl_set_c_,
_drv_err_,
("Set SSID is the same ssid, fw_state
= 0x%08x\n",
get_fwstate(pmlmepriv)));
--
2.19.0
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH v2 8/8] staging: rtl8188eu: simplify function comments
Simplify function comments to a single line.
Signed-off-by: Michael Straube
---
drivers/staging/rtl8188eu/core/rtw_ioctl_set.c | 15 ++-
1 file changed, 2 insertions(+), 13 deletions(-)
diff --git a/drivers/staging/rtl8188eu/core/rtw_ioctl_set.c
b/drivers/staging/rtl8188eu/core/rtw_ioctl_set.c
index c291d3a0fdcc..0b3eb0b40975 100644
--- a/drivers/staging/rtl8188eu/core/rtw_ioctl_set.c
+++ b/drivers/staging/rtl8188eu/core/rtw_ioctl_set.c
@@ -530,12 +530,7 @@ u8 rtw_set_802_11_add_wep(struct adapter *padapter, struct
ndis_802_11_wep *wep)
return ret;
}
-/*
-* rtw_get_cur_max_rate -
-* @adapter: pointer to struct adapter structure
-*
-* Return 0 or 100Kbps
-*/
+/* Return 0 or 100Kbps */
u16 rtw_get_cur_max_rate(struct adapter *adapter)
{
int i = 0;
@@ -586,13 +581,7 @@ u16 rtw_get_cur_max_rate(struct adapter *adapter)
return max_rate;
}
-/*
-* rtw_set_country -
-* @adapter: pointer to struct adapter structure
-* @country_code: string of country code
-*
-* Return _SUCCESS or _FAIL
-*/
+/* Return _SUCCESS or _FAIL */
int rtw_set_country(struct adapter *adapter, const char *country_code)
{
int i;
--
2.19.0
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH v2 6/8] staging: rtl8188eu: add missing spaces around operators
Add missing spaces around '|', '-', and '&' to follow kernel coding
style. Reported by checkpatch.
Signed-off-by: Michael Straube
---
drivers/staging/rtl8188eu/core/rtw_ioctl_set.c | 14 +++---
1 file changed, 7 insertions(+), 7 deletions(-)
diff --git a/drivers/staging/rtl8188eu/core/rtw_ioctl_set.c
b/drivers/staging/rtl8188eu/core/rtw_ioctl_set.c
index db983f15ddd6..d6d6a232c4ec 100644
--- a/drivers/staging/rtl8188eu/core/rtw_ioctl_set.c
+++ b/drivers/staging/rtl8188eu/core/rtw_ioctl_set.c
@@ -141,7 +141,7 @@ u8 rtw_set_802_11_bssid(struct adapter *padapter, u8 *bssid)
else if (check_fwstate(pmlmepriv, _FW_UNDER_LINKING))
goto release_mlme_lock;
- if (check_fwstate(pmlmepriv, _FW_LINKED|WIFI_ADHOC_MASTER_STATE)) {
+ if (check_fwstate(pmlmepriv, _FW_LINKED | WIFI_ADHOC_MASTER_STATE)) {
RT_TRACE(_module_rtl871x_ioctl_set_c_, _drv_info_, ("set_bssid:
_FW_LINKED||WIFI_ADHOC_MASTER_STATE\n"));
if (!memcmp(&pmlmepriv->cur_network.network.MacAddress, bssid,
ETH_ALEN)) {
@@ -225,7 +225,7 @@ u8 rtw_set_802_11_ssid(struct adapter *padapter, struct
ndis_802_11_ssid *ssid)
else if (check_fwstate(pmlmepriv, _FW_UNDER_LINKING))
goto release_mlme_lock;
- if (check_fwstate(pmlmepriv, _FW_LINKED|WIFI_ADHOC_MASTER_STATE)) {
+ if (check_fwstate(pmlmepriv, _FW_LINKED | WIFI_ADHOC_MASTER_STATE)) {
RT_TRACE(_module_rtl871x_ioctl_set_c_, _drv_info_,
("set_ssid: _FW_LINKED||WIFI_ADHOC_MASTER_STATE\n"));
@@ -409,14 +409,14 @@ u8 rtw_set_802_11_bssid_list_scan(struct adapter
*padapter, struct ndis_802_11_s
goto exit;
}
- if (check_fwstate(pmlmepriv, _FW_UNDER_SURVEY|_FW_UNDER_LINKING) ||
+ if (check_fwstate(pmlmepriv, _FW_UNDER_SURVEY | _FW_UNDER_LINKING) ||
pmlmepriv->LinkDetectInfo.bBusyTraffic) {
/* Scan or linking is in progress, do nothing. */
RT_TRACE(_module_rtl871x_ioctl_set_c_, _drv_err_, ("%s fail
since fw_state = %x\n", __func__, get_fwstate(pmlmepriv)));
res = true;
if (check_fwstate(pmlmepriv,
- _FW_UNDER_SURVEY|_FW_UNDER_LINKING))
+ _FW_UNDER_SURVEY | _FW_UNDER_LINKING))
RT_TRACE(_module_rtl871x_ioctl_set_c_, _drv_err_,
("\n###_FW_UNDER_SURVEY|_FW_UNDER_LINKING\n\n"));
else
RT_TRACE(_module_rtl871x_ioctl_set_c_, _drv_err_,
("\n###pmlmepriv->sitesurveyctrl.traffic_busy == true\n\n"));
@@ -550,8 +550,8 @@ u16 rtw_get_cur_max_rate(struct adapter *adapter)
!check_fwstate(pmlmepriv, WIFI_ADHOC_MASTER_STATE))
return 0;
- if (pmlmeext->cur_wireless_mode & (WIRELESS_11_24N|WIRELESS_11_5N)) {
- p = rtw_get_ie(&pcur_bss->ies[12], _HT_CAPABILITY_IE_,
&ht_ielen, pcur_bss->ie_length-12);
+ if (pmlmeext->cur_wireless_mode & (WIRELESS_11_24N | WIRELESS_11_5N)) {
+ p = rtw_get_ie(&pcur_bss->ies[12], _HT_CAPABILITY_IE_,
&ht_ielen, pcur_bss->ie_length - 12);
if (p && ht_ielen > 0) {
/* cur_bwmod is updated by beacon, pmlmeinfo is updated
by association response */
bw_40MHz = (pmlmeext->cur_bwmode &&
(HT_INFO_HT_PARAM_REC_TRANS_CHNL_WIDTH & pmlmeinfo->HT_info.infos[0])) ? 1 : 0;
@@ -569,7 +569,7 @@ u16 rtw_get_cur_max_rate(struct adapter *adapter)
}
} else {
while (pcur_bss->SupportedRates[i] != 0 &&
pcur_bss->SupportedRates[i] != 0xFF) {
- rate = pcur_bss->SupportedRates[i]&0x7F;
+ rate = pcur_bss->SupportedRates[i] & 0x7F;
if (rate > max_rate)
max_rate = rate;
i++;
--
2.19.0
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH v2 2/8] staging: rtl8188eu: remove unnecessary parentheses
Remove unnecessary parentheses as reported by checkpatch
and from conditionals.
Signed-off-by: Michael Straube
---
.../staging/rtl8188eu/core/rtw_ioctl_set.c| 50 +--
1 file changed, 25 insertions(+), 25 deletions(-)
diff --git a/drivers/staging/rtl8188eu/core/rtw_ioctl_set.c
b/drivers/staging/rtl8188eu/core/rtw_ioctl_set.c
index 0880f18520a0..9b67f33cb568 100644
--- a/drivers/staging/rtl8188eu/core/rtw_ioctl_set.c
+++ b/drivers/staging/rtl8188eu/core/rtw_ioctl_set.c
@@ -17,11 +17,11 @@ u8 rtw_do_join(struct adapter *padapter)
{
struct list_head *plist, *phead;
u8 *pibss = NULL;
- struct mlme_priv *pmlmepriv = &(padapter->mlmepriv);
- struct __queue *queue = &(pmlmepriv->scanned_queue);
+ struct mlme_priv *pmlmepriv = &padapter->mlmepriv;
+ struct __queue *queue = &pmlmepriv->scanned_queue;
u8 ret = _SUCCESS;
- spin_lock_bh(&(pmlmepriv->scanned_queue.lock));
+ spin_lock_bh(&pmlmepriv->scanned_queue.lock);
phead = get_list_head(queue);
plist = phead->next;
@@ -36,7 +36,7 @@ u8 rtw_do_join(struct adapter *padapter)
pmlmepriv->to_join = true;
if (list_empty(&queue->queue)) {
- spin_unlock_bh(&(pmlmepriv->scanned_queue.lock));
+ spin_unlock_bh(&pmlmepriv->scanned_queue.lock);
_clr_fwstate_(pmlmepriv, _FW_UNDER_LINKING);
/* when set_ssid/set_bssid for rtw_do_join(), but scanning
queue is empty */
@@ -60,7 +60,7 @@ u8 rtw_do_join(struct adapter *padapter)
} else {
int select_ret;
- spin_unlock_bh(&(pmlmepriv->scanned_queue.lock));
+ spin_unlock_bh(&pmlmepriv->scanned_queue.lock);
select_ret = rtw_select_and_join_from_scanned_queue(pmlmepriv);
if (select_ret == _SUCCESS) {
pmlmepriv->to_join = false;
@@ -71,7 +71,7 @@ u8 rtw_do_join(struct adapter *padapter)
/* submit createbss_cmd to change to a
ADHOC_MASTER */
/* pmlmepriv->lock has been acquired by
caller... */
- struct wlan_bssid_ex*pdev_network =
&(padapter->registrypriv.dev_network);
+ struct wlan_bssid_ex*pdev_network =
&padapter->registrypriv.dev_network;
pmlmepriv->fw_state = WIFI_ADHOC_MASTER_STATE;
@@ -172,7 +172,7 @@ u8 rtw_set_802_11_bssid(struct adapter *padapter, u8 *bssid)
if (padapter->securitypriv.btkip_countermeasure) {
cur_time = jiffies;
- if ((cur_time -
padapter->securitypriv.btkip_countermeasure_time) > 60 * HZ) {
+ if (cur_time - padapter->securitypriv.btkip_countermeasure_time
> 60 * HZ) {
padapter->securitypriv.btkip_countermeasure = false;
padapter->securitypriv.btkip_countermeasure_time = 0;
} else {
@@ -229,8 +229,8 @@ u8 rtw_set_802_11_ssid(struct adapter *padapter, struct
ndis_802_11_ssid *ssid)
RT_TRACE(_module_rtl871x_ioctl_set_c_, _drv_info_,
("set_ssid: _FW_LINKED||WIFI_ADHOC_MASTER_STATE\n"));
- if ((pmlmepriv->assoc_ssid.SsidLength == ssid->SsidLength) &&
- (!memcmp(&pmlmepriv->assoc_ssid.Ssid, ssid->Ssid,
ssid->SsidLength))) {
+ if (pmlmepriv->assoc_ssid.SsidLength == ssid->SsidLength &&
+ !memcmp(&pmlmepriv->assoc_ssid.Ssid, ssid->Ssid,
ssid->SsidLength)) {
if ((check_fwstate(pmlmepriv, WIFI_STATION_STATE) ==
false)) {
RT_TRACE(_module_rtl871x_ioctl_set_c_,
_drv_err_,
("Set SSID is the same ssid, fw_state
= 0x%08x\n",
@@ -279,7 +279,7 @@ u8 rtw_set_802_11_ssid(struct adapter *padapter, struct
ndis_802_11_ssid *ssid)
if (padapter->securitypriv.btkip_countermeasure) {
cur_time = jiffies;
- if ((cur_time -
padapter->securitypriv.btkip_countermeasure_time) > 60 * HZ) {
+ if (cur_time - padapter->securitypriv.btkip_countermeasure_time
> 60 * HZ) {
padapter->securitypriv.btkip_countermeasure = false;
padapter->securitypriv.btkip_countermeasure_time = 0;
} else {
@@ -310,7 +310,7 @@ u8 rtw_set_802_11_infrastructure_mode(struct adapter
*padapter,
{
struct mlme_priv *pmlmepriv = &padapter->mlmepriv;
struct wlan_network*cur_network = &pmlmepriv->cur_network;
- enum ndis_802_11_network_infra *pold_state =
&(cur_network->network.InfrastructureMode);
+ enum ndis_802_11_network_infra *pold_state =
&cur_network->network.InfrastructureMode;
RT_TRACE(_module_rtl871x_ioctl_set_c_, _drv_notice_,
("+rtw_set_802_11_infrastructure_mode: old =%d new =%d
[PATCH v2 3/8] staging: rtl8188eu: remove whitespace
Replace tabs with spaces or just remove spaces in declarations.
Signed-off-by: Michael Straube
---
.../staging/rtl8188eu/core/rtw_ioctl_set.c| 34 +--
1 file changed, 17 insertions(+), 17 deletions(-)
diff --git a/drivers/staging/rtl8188eu/core/rtw_ioctl_set.c
b/drivers/staging/rtl8188eu/core/rtw_ioctl_set.c
index 9b67f33cb568..a00272540846 100644
--- a/drivers/staging/rtl8188eu/core/rtw_ioctl_set.c
+++ b/drivers/staging/rtl8188eu/core/rtw_ioctl_set.c
@@ -17,8 +17,8 @@ u8 rtw_do_join(struct adapter *padapter)
{
struct list_head *plist, *phead;
u8 *pibss = NULL;
- struct mlme_priv *pmlmepriv = &padapter->mlmepriv;
- struct __queue *queue = &pmlmepriv->scanned_queue;
+ struct mlme_priv *pmlmepriv = &padapter->mlmepriv;
+ struct __queue *queue = &pmlmepriv->scanned_queue;
u8 ret = _SUCCESS;
spin_lock_bh(&pmlmepriv->scanned_queue.lock);
@@ -308,8 +308,8 @@ u8 rtw_set_802_11_ssid(struct adapter *padapter, struct
ndis_802_11_ssid *ssid)
u8 rtw_set_802_11_infrastructure_mode(struct adapter *padapter,
enum ndis_802_11_network_infra networktype)
{
- struct mlme_priv *pmlmepriv = &padapter->mlmepriv;
- struct wlan_network*cur_network = &pmlmepriv->cur_network;
+ struct mlme_priv *pmlmepriv = &padapter->mlmepriv;
+ struct wlan_network *cur_network = &pmlmepriv->cur_network;
enum ndis_802_11_network_infra *pold_state =
&cur_network->network.InfrastructureMode;
RT_TRACE(_module_rtl871x_ioctl_set_c_, _drv_notice_,
@@ -394,8 +394,8 @@ u8 rtw_set_802_11_disassociate(struct adapter *padapter)
u8 rtw_set_802_11_bssid_list_scan(struct adapter *padapter, struct
ndis_802_11_ssid *pssid, int ssid_max_num)
{
- struct mlme_priv *pmlmepriv = &padapter->mlmepriv;
- u8 res = true;
+ struct mlme_priv *pmlmepriv = &padapter->mlmepriv;
+ u8 res = true;
RT_TRACE(_module_rtl871x_ioctl_set_c_, _drv_err_, ("+%s(), fw_state
=%x\n", __func__, get_fwstate(pmlmepriv)));
@@ -467,9 +467,9 @@ u8 rtw_set_802_11_authentication_mode(struct adapter
*padapter, enum ndis_802_11
u8 rtw_set_802_11_add_wep(struct adapter *padapter, struct ndis_802_11_wep
*wep)
{
- int keyid, res;
+ int keyid, res;
struct security_priv *psecuritypriv = &padapter->securitypriv;
- u8 ret = _SUCCESS;
+ u8 ret = _SUCCESS;
keyid = wep->KeyIndex & 0x3fff;
@@ -535,16 +535,16 @@ u8 rtw_set_802_11_add_wep(struct adapter *padapter,
struct ndis_802_11_wep *wep)
*/
u16 rtw_get_cur_max_rate(struct adapter *adapter)
{
- int i = 0;
- u8 *p;
- u16 rate = 0, max_rate = 0;
- struct mlme_ext_priv*pmlmeext = &adapter->mlmeextpriv;
- struct mlme_ext_info*pmlmeinfo = &pmlmeext->mlmext_info;
+ int i = 0;
+ u8 *p;
+ u16 rate = 0, max_rate = 0;
+ struct mlme_ext_priv *pmlmeext = &adapter->mlmeextpriv;
+ struct mlme_ext_info *pmlmeinfo = &pmlmeext->mlmext_info;
struct registry_priv *pregistrypriv = &adapter->registrypriv;
- struct mlme_priv*pmlmepriv = &adapter->mlmepriv;
- struct wlan_bssid_ex *pcur_bss = &pmlmepriv->cur_network.network;
- u8 bw_40MHz = 0, short_GI_20 = 0, short_GI_40 = 0;
- u32 ht_ielen = 0;
+ struct mlme_priv *pmlmepriv = &adapter->mlmepriv;
+ struct wlan_bssid_ex *pcur_bss = &pmlmepriv->cur_network.network;
+ u8 bw_40MHz = 0, short_GI_20 = 0, short_GI_40 = 0;
+ u32 ht_ielen = 0;
if (!check_fwstate(pmlmepriv, _FW_LINKED) &&
!check_fwstate(pmlmepriv, WIFI_ADHOC_MASTER_STATE))
--
2.19.0
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [patch 09/11] x86/vdso: Simplify the invalid vclock case
On Tue, Sep 18, 2018 at 09:52:26AM +0200, Thomas Gleixner wrote: > On Mon, 17 Sep 2018, John Stultz wrote: > > On Mon, Sep 17, 2018 at 12:25 PM, Andy Lutomirski wrote: > > > Also, I'm not entirely convinced that this "last" thing is needed at > > > all. John, what's the scenario under which we need it? > > > > So my memory is probably a bit foggy, but I recall that as we > > accelerated gettimeofday, we found that even on systems that claimed > > to have synced TSCs, they were actually just slightly out of sync. > > Enough that right after cycles_last had been updated, a read on > > another cpu could come in just behind cycles_last, resulting in a > > negative interval causing lots of havoc. > > > > So the sanity check is needed to avoid that case. > > Your memory serves you right. That's indeed observable on CPUs which > lack TSC_ADJUST. But, if the gtod code can observe this, then why doesn't the code that checks the sync? ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [patch 09/11] x86/vdso: Simplify the invalid vclock case
On Tue, 18 Sep 2018, Peter Zijlstra wrote: > On Tue, Sep 18, 2018 at 09:52:26AM +0200, Thomas Gleixner wrote: > > On Mon, 17 Sep 2018, John Stultz wrote: > > > On Mon, Sep 17, 2018 at 12:25 PM, Andy Lutomirski wrote: > > > > Also, I'm not entirely convinced that this "last" thing is needed at > > > > all. John, what's the scenario under which we need it? > > > > > > So my memory is probably a bit foggy, but I recall that as we > > > accelerated gettimeofday, we found that even on systems that claimed > > > to have synced TSCs, they were actually just slightly out of sync. > > > Enough that right after cycles_last had been updated, a read on > > > another cpu could come in just behind cycles_last, resulting in a > > > negative interval causing lots of havoc. > > > > > > So the sanity check is needed to avoid that case. > > > > Your memory serves you right. That's indeed observable on CPUs which > > lack TSC_ADJUST. > > But, if the gtod code can observe this, then why doesn't the code that > checks the sync? Because it depends where the involved CPUs are in the topology. The sync code might just run on the same package an simply not see it. Yes, w/o TSC_ADJUST the TSC sync code can just fail to see the havoc. Thanks, tglx ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [patch 09/11] x86/vdso: Simplify the invalid vclock case
On Tue, 18 Sep 2018, Thomas Gleixner wrote: > On Tue, 18 Sep 2018, Peter Zijlstra wrote: > > > Your memory serves you right. That's indeed observable on CPUs which > > > lack TSC_ADJUST. > > > > But, if the gtod code can observe this, then why doesn't the code that > > checks the sync? > > Because it depends where the involved CPUs are in the topology. The sync > code might just run on the same package an simply not see it. Yes, w/o > TSC_ADJUST the TSC sync code can just fail to see the havoc. Even with TSC adjust the TSC can be slightly off by design on multi-socket systems. Thanks, tglx ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [patch 09/11] x86/vdso: Simplify the invalid vclock case
On Tue, 18 Sep 2018, Thomas Gleixner wrote:
> On Tue, 18 Sep 2018, Thomas Gleixner wrote:
> > On Tue, 18 Sep 2018, Peter Zijlstra wrote:
> > > > Your memory serves you right. That's indeed observable on CPUs which
> > > > lack TSC_ADJUST.
> > >
> > > But, if the gtod code can observe this, then why doesn't the code that
> > > checks the sync?
> >
> > Because it depends where the involved CPUs are in the topology. The sync
> > code might just run on the same package an simply not see it. Yes, w/o
> > TSC_ADJUST the TSC sync code can just fail to see the havoc.
>
> Even with TSC adjust the TSC can be slightly off by design on multi-socket
> systems.
Here are the gory details:
https://lore.kernel.org/lkml/3c1737210708230408i7a8049a9m5db49e6c4d89a...@mail.gmail.com/
The changelog has an explanation as well.
d8bb6f4c1670 ("x86: tsc prevent time going backwards")
I still have one of the machines which is affected by this.
Thanks,
tglx
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH v2 1/6] staging: erofs: code cleanup for erofs_kmalloc()
On 2018/9/17 23:34, Chengguang Xu wrote: > Define a dummy function of time_to_inject()/erofs_show_injection_info(), > so that we don't have to check macro CONFIG_EROFS_FAULT_INJECTION in > calling place. > > Signed-off-by: Chengguang Xu Reviewed-by: Chao Yu Thanks, ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH v2 2/6] staging: erofs: code cleanup for option parsing of fault_injection
On 2018/9/17 23:34, Chengguang Xu wrote:
> Define a dummpy function of erofs_build_fault_attr() when macro
> CONFIG_EROFS_FAULT_INJECTION is disabled, so that we don't have to
> check the macro in calling place. Based on above adjustment,
> do proper code cleanup for option parsing of fault_injection.
>
> Signed-off-by: Chengguang Xu
> ---
> drivers/staging/erofs/super.c | 33 -
> 1 file changed, 20 insertions(+), 13 deletions(-)
>
> diff --git a/drivers/staging/erofs/super.c b/drivers/staging/erofs/super.c
> index 9e421536cbdf..7ce2fd3d49f3 100644
> --- a/drivers/staging/erofs/super.c
> +++ b/drivers/staging/erofs/super.c
> @@ -145,10 +145,14 @@ char *erofs_fault_name[FAULT_MAX] = {
> [FAULT_KMALLOC] = "kmalloc",
> };
>
> -static void erofs_build_fault_attr(struct erofs_sb_info *sbi,
> - unsigned int rate)
> +static int erofs_build_fault_attr(struct erofs_sb_info *sbi,
> + substring_t *args)
> {
> struct erofs_fault_info *ffi = &sbi->fault_info;
> + int rate = 0;
> +
> + if (args->from && match_int(args, &rate))
> + return -EINVAL;
>
> if (rate) {
> atomic_set(&ffi->inject_ops, 0);
> @@ -157,6 +161,15 @@ static void erofs_build_fault_attr(struct erofs_sb_info
> *sbi,
> } else {
> memset(ffi, 0, sizeof(struct erofs_fault_info));
> }
> +
> + set_opt(sbi, FAILt_INJECTION);
As Xiang mentioned, it needs to fix this, otherwise, it looks good to me. :)
Thanks,
> +}
> +#else
> +static int erofs_build_fault_attr(struct erofs_sb_info *sbi,
> + substring_t *args)
> +{
> + infoln("fault_injection options not supported");
> + return 0;
> }
> #endif
>
> @@ -193,7 +206,7 @@ static int parse_options(struct super_block *sb, char
> *options)
> {
> substring_t args[MAX_OPT_ARGS];
> char *p;
> - int arg = 0;
> + int err;
>
> if (!options)
> return 0;
> @@ -238,18 +251,12 @@ static int parse_options(struct super_block *sb, char
> *options)
> infoln("noacl options not supported");
> break;
> #endif
> -#ifdef CONFIG_EROFS_FAULT_INJECTION
> - case Opt_fault_injection:
> - if (args->from && match_int(args, &arg))
> - return -EINVAL;
> - erofs_build_fault_attr(EROFS_SB(sb), arg);
> - set_opt(EROFS_SB(sb), FAULT_INJECTION);
> - break;
> -#else
> case Opt_fault_injection:
> - infoln("fault_injection options not supported");
> + err = erofs_build_fault_attr(EROFS_SB(sb), args);
> + if (err)
> + return err;
> break;
> -#endif
> +
> default:
> errln("Unrecognized mount option \"%s\" "
> "or missing value", p);
>
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH v2 2/6] staging: erofs: code cleanup for option parsing of fault_injection
On 09/18/2018 03:07 PM, Gao Xiang wrote:
Hi Chengguang,
On 2018/9/17 23:34, Chengguang Xu wrote:
Define a dummpy function of erofs_build_fault_attr() when macro
CONFIG_EROFS_FAULT_INJECTION is disabled, so that we don't have to
check the macro in calling place. Based on above adjustment,
do proper code cleanup for option parsing of fault_injection.
Signed-off-by: Chengguang Xu
---
drivers/staging/erofs/super.c | 33 -
1 file changed, 20 insertions(+), 13 deletions(-)
diff --git a/drivers/staging/erofs/super.c b/drivers/staging/erofs/super.c
index 9e421536cbdf..7ce2fd3d49f3 100644
--- a/drivers/staging/erofs/super.c
+++ b/drivers/staging/erofs/super.c
@@ -145,10 +145,14 @@ char *erofs_fault_name[FAULT_MAX] = {
[FAULT_KMALLOC] = "kmalloc",
};
-static void erofs_build_fault_attr(struct erofs_sb_info *sbi,
- unsigned int rate)
+static int erofs_build_fault_attr(struct erofs_sb_info *sbi,
+ substring_t *args)
{
struct erofs_fault_info *ffi = &sbi->fault_info;
+ int rate = 0;
+
+ if (args->from && match_int(args, &rate))
+ return -EINVAL;
if (rate) {
atomic_set(&ffi->inject_ops, 0);
@@ -157,6 +161,15 @@ static void erofs_build_fault_attr(struct erofs_sb_info
*sbi,
} else {
memset(ffi, 0, sizeof(struct erofs_fault_info));
}
+
+ set_opt(sbi, FAILt_INJECTION);
drivers/staging/erofs/super.c: In function ‘__erofs_build_fault_attr’:
drivers/staging/erofs/internal.h:176:51: error: ‘EROFS_MOUNT_FAILt_INJECTION’
undeclared (first use in this function)
#define set_opt(sbi, option) ((sbi)->mount_opt |= EROFS_MOUNT_##option)
^
drivers/staging/erofs/super.c:166:2: note: in expansion of macro ‘set_opt’
set_opt(sbi, FAILt_INJECTION);
^
drivers/staging/erofs/internal.h:176:51: note: each undeclared identifier is
reported only once for each function it appears in
#define set_opt(sbi, option) ((sbi)->mount_opt |= EROFS_MOUNT_##option)
^
drivers/staging/erofs/super.c:166:2: note: in expansion of macro ‘set_opt’
set_opt(sbi, FAILt_INJECTION);
^
Hi Xiang,
I'm really sorry for that, I'm curious how it passed my building test.
I deleted all existing config and binary files and tested with/without
INJECTION config this time.
Thanks,
Chengguang
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH v2 3/6] staging: erofs: introduce a new helper __erofs_build_fault_attr()
On 2018/9/17 23:34, Chengguang Xu wrote: > Introduce a new helper __erofs_build_fault_attr() to handle set/clear > erofs_fault_info, we need this funciton for internal use case. > for example, reset fault_injection option in remount. > > Signed-off-by: Chengguang Xu Since this patch is related to your [PATCH 6/6], so I think they can be merge to one, so the purpose of patch can be more clear. Anyway, it's not a big deal. you can add: Reviewed-by: Chao Yu Thanks, ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH v2 4/6] staging: erofs: introduce a new helper erofs_get_fault_rate()
On 2018/9/17 23:34, Chengguang Xu wrote: > Introduce a new helper for getting fault rate, so that we > don't have to check macro in calling place. > > Signed-off-by: Chengguang Xu Can you fold this patch into [5/6]? Thanks, ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH v2 6/6] staging: erofs: option validation in remount
On 2018/9/17 23:34, Chengguang Xu wrote: > Add option validation in remount. After this patch, remount > can change recognized options, and for unknown options remount > will fail and report error. > > Signed-off-by: Chengguang Xu Reviewed-by: Chao Yu Thanks, ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH 0/8] staging: erofs: error handing and more tracepoints
On Fri, Sep 14, 2018 at 10:40:22PM +0800, Gao Xiang wrote: > In order to avoid conflicts with cleanup patches, Chao and I think > it is better to send reviewed preview patches in the erofs mailing list > to the community in time. > > So here is reviewed & tested patches right now, which clean up and > enhance the error handing and add some tracepoints for decompression. > > Note that in this patchset, bare use of 'unsigned' and NULL comparison are > also fixed compared with the preview patches according to the previous > discussion in the staging mailing list. I applied this, but I need to go delete it as this patch series adds a build warning to the system: In file included from drivers/staging/erofs/unzip_vle.h:16:0, from drivers/staging/erofs/unzip_vle.c:13: drivers/staging/erofs/unzip_vle.c: In function ‘z_erofs_map_blocks_iter’: drivers/staging/erofs/internal.h:303:34: warning: ‘pblk’ may be used uninitialized in this function [-Wmaybe-uninitialized] #define blknr_to_addr(nr) ((erofs_off_t)(nr) * EROFS_BLKSIZ) ^ drivers/staging/erofs/unzip_vle.c:1574:20: note: ‘pblk’ was declared here erofs_blk_t mblk, pblk; ^~~~ Please fix that up and resend. thanks, greg k-h ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH] staging: mt7621-mmc: Remove do {} while (0) loop for single statement macro
On Sat, Sep 15, 2018 at 06:47:51PM +0530, Nishad Kamdar wrote:
> This patch removes do {} while (0) loop for single statement macros.
> Issue found by checkpatch.
>
> Signed-off-by: Nishad Kamdar
> ---
> drivers/staging/mt7621-mmc/sd.c | 28 +++-
> 1 file changed, 7 insertions(+), 21 deletions(-)
>
> diff --git a/drivers/staging/mt7621-mmc/sd.c b/drivers/staging/mt7621-mmc/sd.c
> index 7474f9ed7b5b..ec12a3a5a926 100644
> --- a/drivers/staging/mt7621-mmc/sd.c
> +++ b/drivers/staging/mt7621-mmc/sd.c
> @@ -104,14 +104,10 @@ static int cd_active_low = 1;
> /* gate means clock power down */
> static int g_clk_gate = 0;
> #define msdc_gate_clock(id) \
> - do { \
> - g_clk_gate &= ~(1 << ((id) + PERI_MSDC0_PDN)); \
> - } while (0)
> + (g_clk_gate &= ~(1 << ((id) + PERI_MSDC0_PDN)))
This should become an inline function, right?
> /* not like power down register. 1 means clock on. */
> #define msdc_ungate_clock(id) \
> - do {\
> - g_clk_gate |= 1 << ((id) + PERI_MSDC0_PDN); \
> - } while (0)
> + (g_clk_gate |= 1 << ((id) + PERI_MSDC0_PDN))
Same here.
>
> // do we need sync object or not
> void msdc_clk_status(int *status)
> @@ -170,9 +166,7 @@ static void msdc_clr_fifo(struct msdc_host *host)
> } while (0)
>
> #define msdc_irq_restore(val) \
> - do {\
> - sdr_set_bits(host->base + MSDC_INTEN, val); \
> - } while (0)
> + (sdr_set_bits(host->base + MSDC_INTEN, val))
Just call the one function where this is used and delete this #define.
Same type of changes for the rest of these as well.
thanks,
greg k-h
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH v7 1/1] staging: mt7621-mmc: Delete IRQ_MSG() and its users.
On Sat, Sep 15, 2018 at 08:28:03AM +0530, Nishad Kamdar wrote:
> This patch removes IRQ_MSG() and its users as currently it is a no-op.
>
> Signed-off-by: Nishad Kamdar
> ---
> Changes in v7:
> - Delete IRQ_MSG() and all its users
> Changes in v6:
> - No change
> Changes in v5:
> - No change
> ---
> drivers/staging/mt7621-mmc/dbg.h | 12
> drivers/staging/mt7621-mmc/sd.c | 19 ---
> 2 files changed, 31 deletions(-)
>
> diff --git a/drivers/staging/mt7621-mmc/dbg.h
> b/drivers/staging/mt7621-mmc/dbg.h
> index 79914d98c573..4ab9f10dccc2 100644
> --- a/drivers/staging/mt7621-mmc/dbg.h
> +++ b/drivers/staging/mt7621-mmc/dbg.h
> @@ -102,18 +102,6 @@ do { \
> } while (0)
> #endif /* end of +++ */
>
> -#if 1
> -//defined CONFIG_MTK_MMC_CD_POLL
> -#define IRQ_MSG(fmt, args...)
> -#else
> -/* PID in ISR in not corrent */
> -#define IRQ_MSG(fmt, args...) \
> -do { \
> - printk(KERN_ERR TAG"%d -> "fmt" <- %s() : L<%d>\n", \
> -host->id, ##args, __FUNCTION__, __LINE__); \
> -} while (0);
> -#endif
> -
> void msdc_debug_proc_init(void);
>
> #if 0 /* --- chhung */
> diff --git a/drivers/staging/mt7621-mmc/sd.c b/drivers/staging/mt7621-mmc/sd.c
> index 7474f9ed7b5b..273593427d3a 100644
> --- a/drivers/staging/mt7621-mmc/sd.c
> +++ b/drivers/staging/mt7621-mmc/sd.c
> @@ -278,8 +278,6 @@ static void msdc_tasklet_card(struct work_struct *work)
> host->mmc->f_max = HOST_MAX_MCLK;
> mmc_detect_change(host->mmc, msecs_to_jiffies(20));
> }
> -
> - IRQ_MSG("card found<%s>", inserted ? "inserted" : "removed");
> #endif
>
> spin_unlock(&host->lock);
> @@ -1638,17 +1636,10 @@ static irqreturn_t msdc_irq(int irq, void *dev_id)
> if (intsts & MSDC_INT_CDSC) {
> if (host->mmc->caps & MMC_CAP_NEEDS_POLL)
> return IRQ_HANDLED;
> - IRQ_MSG("MSDC_INT_CDSC irq<0x%.8x>", intsts);
> schedule_delayed_work(&host->card_delaywork, HZ);
> /* tuning when plug card ? */
> }
>
> - /* sdio interrupt */
> - if (intsts & MSDC_INT_SDIOIRQ) {
> - IRQ_MSG("XXX MSDC_INT_SDIOIRQ"); /* seems not sdio irq */
> - //mmc_signal_sdio_irq(host->mmc);
> - }
> -
> /* transfer complete interrupt */
> if (data != NULL) {
> if (inten & MSDC_INT_XFER_COMPL) {
> @@ -1663,10 +1654,8 @@ static irqreturn_t msdc_irq(int irq, void *dev_id)
> msdc_clr_int();
>
> if (intsts & MSDC_INT_DATTMO) {
> - IRQ_MSG("XXX CMD<%d> MSDC_INT_DATTMO",
> host->mrq->cmd->opcode);
> data->error = -ETIMEDOUT;
> } else if (intsts & MSDC_INT_DATCRCERR) {
> - IRQ_MSG("XXX CMD<%d> MSDC_INT_DATCRCERR,
> SDC_DCRC_STS<0x%x>", host->mrq->cmd->opcode, readl(host->base +
> SDC_DCRC_STS));
> data->error = -EIO;
> }
The {} should be removed here, right?
thanks,
greg k-h
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH v1 1/6] mm/memory_hotplug: make remove_memory() take the device_hotplug_lock
remove_memory() is exported right now but requires the
device_hotplug_lock, which is not exported. So let's provide a variant
that takes the lock and only export that one.
The lock is already held in
arch/powerpc/platforms/pseries/hotplug-memory.c
drivers/acpi/acpi_memhotplug.c
So, let's use the locked variant.
The lock is not held (but taken in)
arch/powerpc/platforms/powernv/memtrace.c
So let's keep using the (now) locked variant.
Apart from that, there are not other users in the tree.
Cc: Benjamin Herrenschmidt
Cc: Paul Mackerras
Cc: Michael Ellerman
Cc: "Rafael J. Wysocki"
Cc: Len Brown
Cc: Rashmica Gupta
Cc: Michael Neuling
Cc: Balbir Singh
Cc: Nathan Fontenot
Cc: John Allen
Cc: Andrew Morton
Cc: Michal Hocko
Cc: Dan Williams
Cc: Joonsoo Kim
Cc: Vlastimil Babka
Cc: Pavel Tatashin
Cc: Greg Kroah-Hartman
Cc: Oscar Salvador
Cc: YASUAKI ISHIMATSU
Cc: Mathieu Malaterre
Reviewed-by: Pavel Tatashin
Signed-off-by: David Hildenbrand
---
arch/powerpc/platforms/powernv/memtrace.c | 2 --
arch/powerpc/platforms/pseries/hotplug-memory.c | 6 +++---
drivers/acpi/acpi_memhotplug.c | 2 +-
include/linux/memory_hotplug.h | 3 ++-
mm/memory_hotplug.c | 9 -
5 files changed, 14 insertions(+), 8 deletions(-)
diff --git a/arch/powerpc/platforms/powernv/memtrace.c
b/arch/powerpc/platforms/powernv/memtrace.c
index 51dc398ae3f7..8f1cd4f3bfd5 100644
--- a/arch/powerpc/platforms/powernv/memtrace.c
+++ b/arch/powerpc/platforms/powernv/memtrace.c
@@ -90,9 +90,7 @@ static bool memtrace_offline_pages(u32 nid, u64 start_pfn,
u64 nr_pages)
walk_memory_range(start_pfn, end_pfn, (void *)MEM_OFFLINE,
change_memblock_state);
- lock_device_hotplug();
remove_memory(nid, start_pfn << PAGE_SHIFT, nr_pages << PAGE_SHIFT);
- unlock_device_hotplug();
return true;
}
diff --git a/arch/powerpc/platforms/pseries/hotplug-memory.c
b/arch/powerpc/platforms/pseries/hotplug-memory.c
index c1578f54c626..b3f54466e25f 100644
--- a/arch/powerpc/platforms/pseries/hotplug-memory.c
+++ b/arch/powerpc/platforms/pseries/hotplug-memory.c
@@ -334,7 +334,7 @@ static int pseries_remove_memblock(unsigned long base,
unsigned int memblock_siz
nid = memory_add_physaddr_to_nid(base);
for (i = 0; i < sections_per_block; i++) {
- remove_memory(nid, base, MIN_MEMORY_BLOCK_SIZE);
+ __remove_memory(nid, base, MIN_MEMORY_BLOCK_SIZE);
base += MIN_MEMORY_BLOCK_SIZE;
}
@@ -423,7 +423,7 @@ static int dlpar_remove_lmb(struct drmem_lmb *lmb)
block_sz = pseries_memory_block_size();
nid = memory_add_physaddr_to_nid(lmb->base_addr);
- remove_memory(nid, lmb->base_addr, block_sz);
+ __remove_memory(nid, lmb->base_addr, block_sz);
/* Update memory regions for memory remove */
memblock_remove(lmb->base_addr, block_sz);
@@ -710,7 +710,7 @@ static int dlpar_add_lmb(struct drmem_lmb *lmb)
rc = dlpar_online_lmb(lmb);
if (rc) {
- remove_memory(nid, lmb->base_addr, block_sz);
+ __remove_memory(nid, lmb->base_addr, block_sz);
dlpar_remove_device_tree_lmb(lmb);
} else {
lmb->flags |= DRCONF_MEM_ASSIGNED;
diff --git a/drivers/acpi/acpi_memhotplug.c b/drivers/acpi/acpi_memhotplug.c
index 6b0d3ef7309c..811148415993 100644
--- a/drivers/acpi/acpi_memhotplug.c
+++ b/drivers/acpi/acpi_memhotplug.c
@@ -282,7 +282,7 @@ static void acpi_memory_remove_memory(struct
acpi_memory_device *mem_device)
nid = memory_add_physaddr_to_nid(info->start_addr);
acpi_unbind_memory_blocks(info);
- remove_memory(nid, info->start_addr, info->length);
+ __remove_memory(nid, info->start_addr, info->length);
list_del(&info->list);
kfree(info);
}
diff --git a/include/linux/memory_hotplug.h b/include/linux/memory_hotplug.h
index 34a28227068d..1f096852f479 100644
--- a/include/linux/memory_hotplug.h
+++ b/include/linux/memory_hotplug.h
@@ -301,6 +301,7 @@ extern bool is_mem_section_removable(unsigned long pfn,
unsigned long nr_pages);
extern void try_offline_node(int nid);
extern int offline_pages(unsigned long start_pfn, unsigned long nr_pages);
extern void remove_memory(int nid, u64 start, u64 size);
+extern void __remove_memory(int nid, u64 start, u64 size);
#else
static inline bool is_mem_section_removable(unsigned long pfn,
@@ -317,6 +318,7 @@ static inline int offline_pages(unsigned long start_pfn,
unsigned long nr_pages)
}
static inline void remove_memory(int nid, u64 start, u64 size) {}
+static inline void __remove_memory(int nid, u64 start, u64 size) {}
#endif /* CONFIG_MEMORY_HOTREMOVE */
extern void __ref free_area_init_core_hotplug(int nid);
@@ -330,7 +332,6 @@ extern void move_pfn_range_to_zon
[PATCH v1 2/6] mm/memory_hotplug: make add_memory() take the device_hotplug_lock
add_memory() currently does not take the device_hotplug_lock, however
is aleady called under the lock from
arch/powerpc/platforms/pseries/hotplug-memory.c
drivers/acpi/acpi_memhotplug.c
to synchronize against CPU hot-remove and similar.
In general, we should hold the device_hotplug_lock when adding memory
to synchronize against online/offline request (e.g. from user space) -
which already resulted in lock inversions due to device_lock() and
mem_hotplug_lock - see 30467e0b3be ("mm, hotplug: fix concurrent memory
hot-add deadlock"). add_memory()/add_memory_resource() will create memory
block devices, so this really feels like the right thing to do.
Holding the device_hotplug_lock makes sure that a memory block device
can really only be accessed (e.g. via .online/.state) from user space,
once the memory has been fully added to the system.
The lock is not held yet in
drivers/xen/balloon.c
arch/powerpc/platforms/powernv/memtrace.c
drivers/s390/char/sclp_cmd.c
drivers/hv/hv_balloon.c
So, let's either use the locked variants or take the lock.
Don't export add_memory_resource(), as it once was exported to be used
by XEN, which is never built as a module. If somebody requires it, we
also have to export a locked variant (as device_hotplug_lock is never
exported).
Cc: Benjamin Herrenschmidt
Cc: Paul Mackerras
Cc: Michael Ellerman
Cc: "Rafael J. Wysocki"
Cc: Len Brown
Cc: Greg Kroah-Hartman
Cc: Boris Ostrovsky
Cc: Juergen Gross
Cc: Nathan Fontenot
Cc: John Allen
Cc: Andrew Morton
Cc: Michal Hocko
Cc: Dan Williams
Cc: Joonsoo Kim
Cc: Vlastimil Babka
Cc: Oscar Salvador
Cc: Mathieu Malaterre
Cc: Pavel Tatashin
Cc: YASUAKI ISHIMATSU
Reviewed-by: Pavel Tatashin
Signed-off-by: David Hildenbrand
---
.../platforms/pseries/hotplug-memory.c| 2 +-
drivers/acpi/acpi_memhotplug.c| 2 +-
drivers/base/memory.c | 9 ++--
drivers/xen/balloon.c | 3 +++
include/linux/memory_hotplug.h| 1 +
mm/memory_hotplug.c | 22 ---
6 files changed, 32 insertions(+), 7 deletions(-)
diff --git a/arch/powerpc/platforms/pseries/hotplug-memory.c
b/arch/powerpc/platforms/pseries/hotplug-memory.c
index b3f54466e25f..2e6f41dc103a 100644
--- a/arch/powerpc/platforms/pseries/hotplug-memory.c
+++ b/arch/powerpc/platforms/pseries/hotplug-memory.c
@@ -702,7 +702,7 @@ static int dlpar_add_lmb(struct drmem_lmb *lmb)
nid = memory_add_physaddr_to_nid(lmb->base_addr);
/* Add the memory */
- rc = add_memory(nid, lmb->base_addr, block_sz);
+ rc = __add_memory(nid, lmb->base_addr, block_sz);
if (rc) {
dlpar_remove_device_tree_lmb(lmb);
return rc;
diff --git a/drivers/acpi/acpi_memhotplug.c b/drivers/acpi/acpi_memhotplug.c
index 811148415993..8fe0960ea572 100644
--- a/drivers/acpi/acpi_memhotplug.c
+++ b/drivers/acpi/acpi_memhotplug.c
@@ -228,7 +228,7 @@ static int acpi_memory_enable_device(struct
acpi_memory_device *mem_device)
if (node < 0)
node = memory_add_physaddr_to_nid(info->start_addr);
- result = add_memory(node, info->start_addr, info->length);
+ result = __add_memory(node, info->start_addr, info->length);
/*
* If the memory block has been used by the kernel, add_memory()
diff --git a/drivers/base/memory.c b/drivers/base/memory.c
index 817320c7c4c1..40cac122ec73 100644
--- a/drivers/base/memory.c
+++ b/drivers/base/memory.c
@@ -519,15 +519,20 @@ memory_probe_store(struct device *dev, struct
device_attribute *attr,
if (phys_addr & ((pages_per_block << PAGE_SHIFT) - 1))
return -EINVAL;
+ ret = lock_device_hotplug_sysfs();
+ if (ret)
+ goto out;
+
nid = memory_add_physaddr_to_nid(phys_addr);
- ret = add_memory(nid, phys_addr,
-MIN_MEMORY_BLOCK_SIZE * sections_per_block);
+ ret = __add_memory(nid, phys_addr,
+ MIN_MEMORY_BLOCK_SIZE * sections_per_block);
if (ret)
goto out;
ret = count;
out:
+ unlock_device_hotplug();
return ret;
}
diff --git a/drivers/xen/balloon.c b/drivers/xen/balloon.c
index e12bb256036f..6bab019a82b1 100644
--- a/drivers/xen/balloon.c
+++ b/drivers/xen/balloon.c
@@ -395,7 +395,10 @@ static enum bp_state reserve_additional_memory(void)
* callers drop the mutex before trying again.
*/
mutex_unlock(&balloon_mutex);
+ /* add_memory_resource() requires the device_hotplug lock */
+ lock_device_hotplug();
rc = add_memory_resource(nid, resource, memhp_auto_online);
+ unlock_device_hotplug();
mutex_lock(&balloon_mutex);
if (rc) {
diff --git a/include/linux/memory_hotplug.h b/include/linux/memory_hotplug.h
index 1f096852f479..
[PATCH v1 0/6] mm: online/offline_pages called w.o. mem_hotplug_lock
Reading through the code and studying how mem_hotplug_lock is to be used, I noticed that there are two places where we can end up calling device_online()/device_offline() - online_pages()/offline_pages() without the mem_hotplug_lock. And there are other places where we call device_online()/device_offline() without the device_hotplug_lock. While e.g. echo "online" > /sys/devices/system/memory/memory9/state is fine, e.g. echo 1 > /sys/devices/system/memory/memory9/online Will not take the mem_hotplug_lock. However the device_lock() and device_hotplug_lock. E.g. via memory_probe_store(), we can end up calling add_memory()->online_pages() without the device_hotplug_lock. So we can have concurrent callers in online_pages(). We e.g. touch in online_pages() basically unprotected zone->present_pages then. Looks like there is a longer history to that (see Patch #2 for details), and fixing it to work the way it was intended is not really possible. We would e.g. have to take the mem_hotplug_lock in device/base/core.c, which sounds wrong. Summary: We had a lock inversion on mem_hotplug_lock and device_lock(). More details can be found in patch 3 and patch 6. I propose the general rules (documentation added in patch 6): 1. add_memory/add_memory_resource() must only be called with device_hotplug_lock. 2. remove_memory() must only be called with device_hotplug_lock. This is already documented and holds for all callers. 3. device_online()/device_offline() must only be called with device_hotplug_lock. This is already documented and true for now in core code. Other callers (related to memory hotplug) have to be fixed up. 4. mem_hotplug_lock is taken inside of add_memory/remove_memory/ online_pages/offline_pages. To me, this looks way cleaner than what we have right now (and easier to verify). And looking at the documentation of remove_memory, using lock_device_hotplug also for add_memory() feels natural. RFCv2 -> v1: - Dropped an unnecessary _ref from remove_memory() in patch #1 - Minor patch description fixes. - Added rb's RFC -> RFCv2: - Don't export device_hotplug_lock, provide proper remove_memory/add_memory wrappers. - Split up the patches a bit. - Try to improve powernv memtrace locking - Add some documentation for locking that matches my knowledge David Hildenbrand (6): mm/memory_hotplug: make remove_memory() take the device_hotplug_lock mm/memory_hotplug: make add_memory() take the device_hotplug_lock mm/memory_hotplug: fix online/offline_pages called w.o. mem_hotplug_lock powerpc/powernv: hold device_hotplug_lock when calling device_online() powerpc/powernv: hold device_hotplug_lock in memtrace_offline_pages() memory-hotplug.txt: Add some details about locking internals Documentation/memory-hotplug.txt | 39 +++- arch/powerpc/platforms/powernv/memtrace.c | 14 +++-- .../platforms/pseries/hotplug-memory.c| 8 +-- drivers/acpi/acpi_memhotplug.c| 4 +- drivers/base/memory.c | 22 +++ drivers/xen/balloon.c | 3 + include/linux/memory_hotplug.h| 4 +- mm/memory_hotplug.c | 59 +++ 8 files changed, 115 insertions(+), 38 deletions(-) -- 2.17.1 ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH v1 3/6] mm/memory_hotplug: fix online/offline_pages called w.o. mem_hotplug_lock
There seem to be some problems as result of 30467e0b3be ("mm, hotplug:
fix concurrent memory hot-add deadlock"), which tried to fix a possible
lock inversion reported and discussed in [1] due to the two locks
a) device_lock()
b) mem_hotplug_lock
While add_memory() first takes b), followed by a) during
bus_probe_device(), onlining of memory from user space first took a),
followed by b), exposing a possible deadlock.
In [1], and it was decided to not make use of device_hotplug_lock, but
rather to enforce a locking order.
The problems I spotted related to this:
1. Memory block device attributes: While .state first calls
mem_hotplug_begin() and the calls device_online() - which takes
device_lock() - .online does no longer call mem_hotplug_begin(), so
effectively calls online_pages() without mem_hotplug_lock.
2. device_online() should be called under device_hotplug_lock, however
onlining memory during add_memory() does not take care of that.
In addition, I think there is also something wrong about the locking in
3. arch/powerpc/platforms/powernv/memtrace.c calls offline_pages()
without locks. This was introduced after 30467e0b3be. And skimming over
the code, I assume it could need some more care in regards to locking
(e.g. device_online() called without device_hotplug_lock. This will
be addressed in the following patches.
Now that we hold the device_hotplug_lock when
- adding memory (e.g. via add_memory()/add_memory_resource())
- removing memory (e.g. via remove_memory())
- device_online()/device_offline()
We can move mem_hotplug_lock usage back into
online_pages()/offline_pages().
Why is mem_hotplug_lock still needed? Essentially to make
get_online_mems()/put_online_mems() be very fast (relying on
device_hotplug_lock would be very slow), and to serialize against
addition of memory that does not create memory block devices (hmm).
[1] http://driverdev.linuxdriverproject.org/pipermail/ driverdev-devel/
2015-February/065324.html
This patch is partly based on a patch by Vitaly Kuznetsov.
Cc: Benjamin Herrenschmidt
Cc: Paul Mackerras
Cc: Michael Ellerman
Cc: "Rafael J. Wysocki"
Cc: Len Brown
Cc: Greg Kroah-Hartman
Cc: "K. Y. Srinivasan"
Cc: Haiyang Zhang
Cc: Stephen Hemminger
Cc: Martin Schwidefsky
Cc: Heiko Carstens
Cc: Boris Ostrovsky
Cc: Juergen Gross
Cc: Rashmica Gupta
Cc: Michael Neuling
Cc: Balbir Singh
Cc: Kate Stewart
Cc: Thomas Gleixner
Cc: Philippe Ombredanne
Cc: Andrew Morton
Cc: Michal Hocko
Cc: Pavel Tatashin
Cc: Vlastimil Babka
Cc: Dan Williams
Cc: Oscar Salvador
Cc: YASUAKI ISHIMATSU
Cc: Mathieu Malaterre
Reviewed-by: Pavel Tatashin
Reviewed-by: Rashmica Gupta
Signed-off-by: David Hildenbrand
---
drivers/base/memory.c | 13 +
mm/memory_hotplug.c | 28
2 files changed, 21 insertions(+), 20 deletions(-)
diff --git a/drivers/base/memory.c b/drivers/base/memory.c
index 40cac122ec73..0e5985682642 100644
--- a/drivers/base/memory.c
+++ b/drivers/base/memory.c
@@ -228,7 +228,6 @@ static bool pages_correctly_probed(unsigned long start_pfn)
/*
* MEMORY_HOTPLUG depends on SPARSEMEM in mm/Kconfig, so it is
* OK to have direct references to sparsemem variables in here.
- * Must already be protected by mem_hotplug_begin().
*/
static int
memory_block_action(unsigned long phys_index, unsigned long action, int
online_type)
@@ -294,7 +293,6 @@ static int memory_subsys_online(struct device *dev)
if (mem->online_type < 0)
mem->online_type = MMOP_ONLINE_KEEP;
- /* Already under protection of mem_hotplug_begin() */
ret = memory_block_change_state(mem, MEM_ONLINE, MEM_OFFLINE);
/* clear online_type */
@@ -341,19 +339,11 @@ store_mem_state(struct device *dev,
goto err;
}
- /*
-* Memory hotplug needs to hold mem_hotplug_begin() for probe to find
-* the correct memory block to online before doing device_online(dev),
-* which will take dev->mutex. Take the lock early to prevent an
-* inversion, memory_subsys_online() callbacks will be implemented by
-* assuming it's already protected.
-*/
- mem_hotplug_begin();
-
switch (online_type) {
case MMOP_ONLINE_KERNEL:
case MMOP_ONLINE_MOVABLE:
case MMOP_ONLINE_KEEP:
+ /* mem->online_type is protected by device_hotplug_lock */
mem->online_type = online_type;
ret = device_online(&mem->dev);
break;
@@ -364,7 +354,6 @@ store_mem_state(struct device *dev,
ret = -EINVAL; /* should never happen */
}
- mem_hotplug_done();
err:
unlock_device_hotplug();
diff --git a/mm/memory_hotplug.c b/mm/memory_hotplug.c
index ef5444145c88..497e9315ca6f 100644
--- a/mm/memory_hotplug.c
+++ b/mm/memory_hotplug.c
@@ -881,7 +881,6 @@ static struct zone * __meminit move_pfn_range(int
online_type, int ni
[PATCH v1 6/6] memory-hotplug.txt: Add some details about locking internals
Let's document the magic a bit, especially why device_hotplug_lock is required when adding/removing memory and how it all play together with requests to online/offline memory from user space. Cc: Jonathan Corbet Cc: Michal Hocko Cc: Andrew Morton Reviewed-by: Pavel Tatashin Signed-off-by: David Hildenbrand --- Documentation/memory-hotplug.txt | 39 +++- 1 file changed, 38 insertions(+), 1 deletion(-) diff --git a/Documentation/memory-hotplug.txt b/Documentation/memory-hotplug.txt index 7f49ebf3ddb2..03aaad7d7373 100644 --- a/Documentation/memory-hotplug.txt +++ b/Documentation/memory-hotplug.txt @@ -3,7 +3,7 @@ Memory Hotplug == :Created: Jul 28 2007 -:Updated: Add description of notifier of memory hotplug: Oct 11 2007 +:Updated: Add some details about locking internals:Aug 20 2018 This document is about memory hotplug including how-to-use and current status. Because Memory Hotplug is still under development, contents of this text will @@ -495,6 +495,43 @@ further processing of the notification queue. NOTIFY_STOP stops further processing of the notification queue. + +Locking Internals += + +When adding/removing memory that uses memory block devices (i.e. ordinary RAM), +the device_hotplug_lock should be held to: + +- synchronize against online/offline requests (e.g. via sysfs). This way, memory + block devices can only be accessed (.online/.state attributes) by user + space once memory has been fully added. And when removing memory, we + know nobody is in critical sections. +- synchronize against CPU hotplug and similar (e.g. relevant for ACPI and PPC) + +Especially, there is a possible lock inversion that is avoided using +device_hotplug_lock when adding memory and user space tries to online that +memory faster than expected: + +- device_online() will first take the device_lock(), followed by + mem_hotplug_lock +- add_memory_resource() will first take the mem_hotplug_lock, followed by + the device_lock() (while creating the devices, during bus_add_device()). + +As the device is visible to user space before taking the device_lock(), this +can result in a lock inversion. + +onlining/offlining of memory should be done via device_online()/ +device_offline() - to make sure it is properly synchronized to actions +via sysfs. Holding device_hotplug_lock is advised (to e.g. protect online_type) + +When adding/removing/onlining/offlining memory or adding/removing +heterogeneous/device memory, we should always hold the mem_hotplug_lock to +serialise memory hotplug (e.g. access to global/zone variables). + +In addition, mem_hotplug_lock (in contrast to device_hotplug_lock) allows +for a quite efficient get_online_mems/put_online_mems implementation. + + Future Work === -- 2.17.1 ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH v1 5/6] powerpc/powernv: hold device_hotplug_lock in memtrace_offline_pages()
Let's perform all checking + offlining + removing under
device_hotplug_lock, so nobody can mess with these devices via
sysfs concurrently.
Cc: Benjamin Herrenschmidt
Cc: Paul Mackerras
Cc: Michael Ellerman
Cc: Rashmica Gupta
Cc: Balbir Singh
Cc: Michael Neuling
Reviewed-by: Pavel Tatashin
Signed-off-by: David Hildenbrand
---
arch/powerpc/platforms/powernv/memtrace.c | 10 --
1 file changed, 8 insertions(+), 2 deletions(-)
diff --git a/arch/powerpc/platforms/powernv/memtrace.c
b/arch/powerpc/platforms/powernv/memtrace.c
index ef7181d4fe68..473e59842ec5 100644
--- a/arch/powerpc/platforms/powernv/memtrace.c
+++ b/arch/powerpc/platforms/powernv/memtrace.c
@@ -74,9 +74,13 @@ static bool memtrace_offline_pages(u32 nid, u64 start_pfn,
u64 nr_pages)
{
u64 end_pfn = start_pfn + nr_pages - 1;
+ lock_device_hotplug();
+
if (walk_memory_range(start_pfn, end_pfn, NULL,
- check_memblock_online))
+ check_memblock_online)) {
+ unlock_device_hotplug();
return false;
+ }
walk_memory_range(start_pfn, end_pfn, (void *)MEM_GOING_OFFLINE,
change_memblock_state);
@@ -84,14 +88,16 @@ static bool memtrace_offline_pages(u32 nid, u64 start_pfn,
u64 nr_pages)
if (offline_pages(start_pfn, nr_pages)) {
walk_memory_range(start_pfn, end_pfn, (void *)MEM_ONLINE,
change_memblock_state);
+ unlock_device_hotplug();
return false;
}
walk_memory_range(start_pfn, end_pfn, (void *)MEM_OFFLINE,
change_memblock_state);
- remove_memory(nid, start_pfn << PAGE_SHIFT, nr_pages << PAGE_SHIFT);
+ __remove_memory(nid, start_pfn << PAGE_SHIFT, nr_pages << PAGE_SHIFT);
+ unlock_device_hotplug();
return true;
}
--
2.17.1
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH v1 4/6] powerpc/powernv: hold device_hotplug_lock when calling device_online()
device_online() should be called with device_hotplug_lock() held.
Cc: Benjamin Herrenschmidt
Cc: Paul Mackerras
Cc: Michael Ellerman
Cc: Rashmica Gupta
Cc: Balbir Singh
Cc: Michael Neuling
Reviewed-by: Pavel Tatashin
Signed-off-by: David Hildenbrand
---
arch/powerpc/platforms/powernv/memtrace.c | 2 ++
1 file changed, 2 insertions(+)
diff --git a/arch/powerpc/platforms/powernv/memtrace.c
b/arch/powerpc/platforms/powernv/memtrace.c
index 8f1cd4f3bfd5..ef7181d4fe68 100644
--- a/arch/powerpc/platforms/powernv/memtrace.c
+++ b/arch/powerpc/platforms/powernv/memtrace.c
@@ -229,9 +229,11 @@ static int memtrace_online(void)
* we need to online the memory ourselves.
*/
if (!memhp_auto_online) {
+ lock_device_hotplug();
walk_memory_range(PFN_DOWN(ent->start),
PFN_UP(ent->start + ent->size - 1),
NULL, online_mem_block);
+ unlock_device_hotplug();
}
/*
--
2.17.1
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH 0/8] staging: erofs: error handing and more tracepoints
Hi Greg, On 2018/9/18 19:19, Greg Kroah-Hartman wrote: > On Fri, Sep 14, 2018 at 10:40:22PM +0800, Gao Xiang wrote: >> In order to avoid conflicts with cleanup patches, Chao and I think >> it is better to send reviewed preview patches in the erofs mailing list >> to the community in time. >> >> So here is reviewed & tested patches right now, which clean up and >> enhance the error handing and add some tracepoints for decompression. >> >> Note that in this patchset, bare use of 'unsigned' and NULL comparison are >> also fixed compared with the preview patches according to the previous >> discussion in the staging mailing list. > > I applied this, but I need to go delete it as this patch series adds a > build warning to the system: > > In file included from drivers/staging/erofs/unzip_vle.h:16:0, > from drivers/staging/erofs/unzip_vle.c:13: > drivers/staging/erofs/unzip_vle.c: In function ‘z_erofs_map_blocks_iter’: > drivers/staging/erofs/internal.h:303:34: warning: ‘pblk’ may be used > uninitialized in this function [-Wmaybe-uninitialized] > #define blknr_to_addr(nr) ((erofs_off_t)(nr) * EROFS_BLKSIZ) > ^ > drivers/staging/erofs/unzip_vle.c:1574:20: note: ‘pblk’ was declared here > erofs_blk_t mblk, pblk; > ^~~~ > > Please fix that up and resend. strange... my compiler (4.8.4) and huawei internal CI don't report that, and this patchset has been in Chao's tree for a while, I don't get any report so far... I just looked into that code again and it seems a false warning since 1) this code is heavily running on the products and working fine till now. 2) pblk gets a proper value before unzip_vle.c:1690 map->m_pa = blknr_to_addr(pblk); so I think I need to silence this warning for now and check if there is a really issue Thanks, Gao Xiang > > thanks, > > greg k-h > ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH 0/8] staging: erofs: error handing and more tracepoints
On Tue, Sep 18, 2018 at 08:02:30PM +0800, Gao Xiang wrote: > Hi Greg, > > On 2018/9/18 19:19, Greg Kroah-Hartman wrote: > > On Fri, Sep 14, 2018 at 10:40:22PM +0800, Gao Xiang wrote: > >> In order to avoid conflicts with cleanup patches, Chao and I think > >> it is better to send reviewed preview patches in the erofs mailing list > >> to the community in time. > >> > >> So here is reviewed & tested patches right now, which clean up and > >> enhance the error handing and add some tracepoints for decompression. > >> > >> Note that in this patchset, bare use of 'unsigned' and NULL comparison are > >> also fixed compared with the preview patches according to the previous > >> discussion in the staging mailing list. > > > > I applied this, but I need to go delete it as this patch series adds a > > build warning to the system: > > > > In file included from drivers/staging/erofs/unzip_vle.h:16:0, > > from drivers/staging/erofs/unzip_vle.c:13: > > drivers/staging/erofs/unzip_vle.c: In function ‘z_erofs_map_blocks_iter’: > > drivers/staging/erofs/internal.h:303:34: warning: ‘pblk’ may be used > > uninitialized in this function [-Wmaybe-uninitialized] > > #define blknr_to_addr(nr) ((erofs_off_t)(nr) * EROFS_BLKSIZ) > > ^ > > drivers/staging/erofs/unzip_vle.c:1574:20: note: ‘pblk’ was declared here > > erofs_blk_t mblk, pblk; > > ^~~~ > > > > Please fix that up and resend. > > strange... my compiler (4.8.4) and huawei internal CI don't report that, > and this patchset has been in Chao's tree for a while, I don't get any report > so far... > > I just looked into that code again and it seems a false warning since > 1) this code is heavily running on the products and working fine till now. > 2) pblk gets a proper value before unzip_vle.c:1690 map->m_pa = > blknr_to_addr(pblk); > > so I think I need to silence this warning for now and check if there is a > really issue Don't just silent the warning. Usually gcc now properly detects where there really is a problem, it should not be a false-positive. And in looking at the code, it does seem that pblk could not be set if one of the different case statements is taken and then exact_hitted: is jumped to, right? Or is gcc just being "dumb" here? What about clang, have you tried building it with that compiler as well? thanks, greg k-h ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH 0/8] staging: erofs: error handing and more tracepoints
Hi Greg,
On 2018/9/18 20:14, Greg Kroah-Hartman wrote:
> On Tue, Sep 18, 2018 at 08:02:30PM +0800, Gao Xiang wrote:
>> Hi Greg,
>>
>> On 2018/9/18 19:19, Greg Kroah-Hartman wrote:
>>> On Fri, Sep 14, 2018 at 10:40:22PM +0800, Gao Xiang wrote:
In order to avoid conflicts with cleanup patches, Chao and I think
it is better to send reviewed preview patches in the erofs mailing list
to the community in time.
So here is reviewed & tested patches right now, which clean up and
enhance the error handing and add some tracepoints for decompression.
Note that in this patchset, bare use of 'unsigned' and NULL comparison are
also fixed compared with the preview patches according to the previous
discussion in the staging mailing list.
>>>
>>> I applied this, but I need to go delete it as this patch series adds a
>>> build warning to the system:
>>>
>>> In file included from drivers/staging/erofs/unzip_vle.h:16:0,
>>> from drivers/staging/erofs/unzip_vle.c:13:
>>> drivers/staging/erofs/unzip_vle.c: In function ‘z_erofs_map_blocks_iter’:
>>> drivers/staging/erofs/internal.h:303:34: warning: ‘pblk’ may be used
>>> uninitialized in this function [-Wmaybe-uninitialized]
>>> #define blknr_to_addr(nr) ((erofs_off_t)(nr) * EROFS_BLKSIZ)
>>> ^
>>> drivers/staging/erofs/unzip_vle.c:1574:20: note: ‘pblk’ was declared here
>>> erofs_blk_t mblk, pblk;
>>> ^~~~
>>>
>>> Please fix that up and resend.
>>
>> strange... my compiler (4.8.4) and huawei internal CI don't report that,
>> and this patchset has been in Chao's tree for a while, I don't get any
>> report so far...
>>
>> I just looked into that code again and it seems a false warning since
>> 1) this code is heavily running on the products and working fine till now.
>> 2) pblk gets a proper value before unzip_vle.c:1690 map->m_pa =
>> blknr_to_addr(pblk);
>>
>> so I think I need to silence this warning for now and check if there is a
>> really issue
>
> Don't just silent the warning. Usually gcc now properly detects where
> there really is a problem, it should not be a false-positive. And in
> looking at the code, it does seem that pblk could not be set if one of
> the different case statements is taken and then exact_hitted: is jumped
> to, right?
pblk is assigned before each "goto exact_hitted" and "break;" here.
1641 switch (cluster_type) {
1642 case Z_EROFS_VLE_CLUSTER_TYPE_PLAIN:
1643 if (ofs_rem >= logical_cluster_ofs)
1644 map->m_flags ^= EROFS_MAP_ZIPPED;
1645 /* fallthrough */
1646 case Z_EROFS_VLE_CLUSTER_TYPE_HEAD:
1647 if (ofs_rem == logical_cluster_ofs) {
1648 pblk = le32_to_cpu(di->di_u.blkaddr);
1649 goto exact_hitted; <--- assigned
1650 }
1651
1652 if (ofs_rem > logical_cluster_ofs) {
1653 ofs = (u64)lcn * clustersize | logical_cluster_ofs;
1654 pblk = le32_to_cpu(di->di_u.blkaddr);
1655 break; <--- assigned
1656 }
1657
1658 /* logical cluster number should be >= 1 */
1659 if (unlikely(!lcn)) {
1660 errln("invalid logical cluster 0 at nid %llu",
1661 EROFS_V(inode)->nid);
1662 err = -EIO;
1663 goto unmap_out; <--- bypass no need it
1664 }
1665 end = ((u64)lcn-- * clustersize) | logical_cluster_ofs;
1666 /* fallthrough */
1667 case Z_EROFS_VLE_CLUSTER_TYPE_NONHEAD:
1668 /* get the correspoinding first chunk */
1669 err = vle_get_logical_extent_head(&ctx, lcn, &ofs,
1670 &pblk, &map->m_flags);
1671 mpage = *mpage_ret;
1672
1673 if (unlikely(err)) {
1674 if (mpage)
1675 goto unmap_out; <--- bypass no need it
1676 goto out; <--- bypass no need it
1677 }
1678 break; <-- pblk should be assigned in
vle_get_logical_extent_head
if no error occurred in
vle_get_logical_extent_head
1679 default:
1680 errln("unknown cluster type %u at offset %llu of nid %llu",
1681 cluster_type, ofs, EROFS_V(inode)->nid);
1682 err = -EIO;
1683 goto unmap_out; <-- bypass no need it
1684 }
1685
1686 map->m_la = ofs;
1687 exact_hitted:
1688 map->m_llen = end - ofs;
1689 map->m_plen = clustersize;
1690 map->m_pa = blknr_to_addr(pblk); <--- pblk is used here
1691 map->m_flags |= EROFS_MAP_MAPPED;
1692 u
[PATCH] staging: olpc_dcon: add a missing dependency
WARNING: unmet direct dependencies detected for BACKLIGHT_CLASS_DEVICE Depends on [n]: HAS_IOMEM [=y] && BACKLIGHT_LCD_SUPPORT [=n] Selected by [y]: - FB_OLPC_DCON [=y] && STAGING [=y] && X86 [=y] && OLPC [=y] && FB [=y] && I2C [=y] && (GPIO_CS5535 [=n] || GPIO_CS5535 [=n]=n) Signed-off-by: Lubomir Rintel --- drivers/staging/olpc_dcon/Kconfig | 1 + 1 file changed, 1 insertion(+) diff --git a/drivers/staging/olpc_dcon/Kconfig b/drivers/staging/olpc_dcon/Kconfig index c91a56f77bcb..192cc8d0853f 100644 --- a/drivers/staging/olpc_dcon/Kconfig +++ b/drivers/staging/olpc_dcon/Kconfig @@ -2,6 +2,7 @@ config FB_OLPC_DCON tristate "One Laptop Per Child Display CONtroller support" depends on OLPC && FB depends on I2C + depends on BACKLIGHT_LCD_SUPPORT depends on (GPIO_CS5535 || GPIO_CS5535=n) select BACKLIGHT_CLASS_DEVICE help -- 2.17.1 ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [patch 09/11] x86/vdso: Simplify the invalid vclock case
On Tue, Sep 18, 2018 at 12:41:57PM +0200, Thomas Gleixner wrote:
> On Tue, 18 Sep 2018, Thomas Gleixner wrote:
> > On Tue, 18 Sep 2018, Thomas Gleixner wrote:
> > > On Tue, 18 Sep 2018, Peter Zijlstra wrote:
> > > > > Your memory serves you right. That's indeed observable on CPUs which
> > > > > lack TSC_ADJUST.
> > > >
> > > > But, if the gtod code can observe this, then why doesn't the code that
> > > > checks the sync?
> > >
> > > Because it depends where the involved CPUs are in the topology. The sync
> > > code might just run on the same package an simply not see it. Yes, w/o
> > > TSC_ADJUST the TSC sync code can just fail to see the havoc.
> >
> > Even with TSC adjust the TSC can be slightly off by design on multi-socket
> > systems.
>
> Here are the gory details:
>
>
> https://lore.kernel.org/lkml/3c1737210708230408i7a8049a9m5db49e6c4d89a...@mail.gmail.com/
>
> The changelog has an explanation as well.
>
> d8bb6f4c1670 ("x86: tsc prevent time going backwards")
>
> I still have one of the machines which is affected by this.
Are we sure this isn't a load vs rdtsc reorder? Because if I look at the
current code:
notrace static u64 vread_tsc(void)
{
u64 ret = (u64)rdtsc_ordered();
u64 last = gtod->cycle_last;
if (likely(ret >= last))
return ret;
/*
* GCC likes to generate cmov here, but this branch is extremely
* predictable (it's just a function of time and the likely is
* very likely) and there's a data dependence, so force GCC
* to generate a branch instead. I don't barrier() because
* we don't actually need a barrier, and if this function
* ever gets inlined it will generate worse code.
*/
asm volatile ("");
return last;
}
That does:
lfence
rdtsc
load gtod->cycle_last
Which obviously allows us to observe a cycles_last that is later than
the rdtsc itself, and thus time can trivially go backwards.
The new code:
last = gtod->cycle_last;
cycles = vgetcyc(gtod->vclock_mode);
if (unlikely((s64)cycles < 0))
return vdso_fallback_gettime(clk, ts);
if (cycles > last)
ns += (cycles - last) * gtod->mult;
looks like:
load gtod->cycle_last
lfence
rdtsc
which avoids that possibility, the cycle_last load must have completed
before the rdtsc.
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH 0/8] staging: erofs: error handing and more tracepoints
On Tue, Sep 18, 2018 at 08:31:22PM +0800, Gao Xiang wrote: > (I have no clang environment to build kernel yet... :( I will try later, but > I have no idea why it happens, > and it seems a false warning). > > Thanks, > Gao Xiang > > > > > Or is gcc just being "dumb" here? What about clang, have you tried > > building it with that compiler as well? Yeah. Gcc is wrong. Gcc used to be extra conservative and missed a bunch of bugs, but I guess now they're going in to opposite direction? Smatch gets this correct, but you have to rebuild the cross function DB twice to make the warning go away. It could be because it starts out with the old vle_get_logical_extent_head() information so it thinks it knows how that function works. regards, dan carpenter ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH 0/8] staging: erofs: error handing and more tracepoints
On Tue, Sep 18, 2018 at 04:03:37PM +0300, Dan Carpenter wrote: > On Tue, Sep 18, 2018 at 08:31:22PM +0800, Gao Xiang wrote: > > (I have no clang environment to build kernel yet... :( I will try later, > > but I have no idea why it happens, > > and it seems a false warning). > > > > Thanks, > > Gao Xiang > > > > > > > > Or is gcc just being "dumb" here? What about clang, have you tried > > > building it with that compiler as well? > > Yeah. Gcc is wrong. Gcc used to be extra conservative and missed a > bunch of bugs, but I guess now they're going in to opposite direction? > > Smatch gets this correct, but you have to rebuild the cross function DB > twice to make the warning go away. It could be because it starts out > with the old vle_get_logical_extent_head() information so it thinks it > knows how that function works. Ok, thanks for checking (both of you). Just initialize the variable to keep gcc from printing foolish warnings. thanks, greg k-h ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH 0/8] staging: erofs: error handing and more tracepoints
On 2018/9/18 21:09, Greg Kroah-Hartman wrote: > On Tue, Sep 18, 2018 at 04:03:37PM +0300, Dan Carpenter wrote: >> On Tue, Sep 18, 2018 at 08:31:22PM +0800, Gao Xiang wrote: >>> (I have no clang environment to build kernel yet... :( I will try later, >>> but I have no idea why it happens, >>> and it seems a false warning). >>> >>> Thanks, >>> Gao Xiang >>> Or is gcc just being "dumb" here? What about clang, have you tried building it with that compiler as well? >> >> Yeah. Gcc is wrong. Gcc used to be extra conservative and missed a >> bunch of bugs, but I guess now they're going in to opposite direction? >> >> Smatch gets this correct, but you have to rebuild the cross function DB >> twice to make the warning go away. It could be because it starts out >> with the old vle_get_logical_extent_head() information so it thinks it >> knows how that function works. Hi Dan, Thanks for taking time to confirm this and the detailed explanation ;) > > Ok, thanks for checking (both of you). Just initialize the variable to > keep gcc from printing foolish warnings. Hi Greg, Ok, if you don't tend to use `uninitialized_var(...)', I will initialize a dumb value...but I think it is really useless to initialize it... :( I will resend the related false warning patch independently soon... Thanks, Gao Xiang > > thanks, > > greg k-h > ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [patch 09/11] x86/vdso: Simplify the invalid vclock case
On Tue, 18 Sep 2018, Peter Zijlstra wrote: > On Tue, Sep 18, 2018 at 12:41:57PM +0200, Thomas Gleixner wrote: > > I still have one of the machines which is affected by this. > > Are we sure this isn't a load vs rdtsc reorder? Because if I look at the > current code: The load order of last vs. rdtsc does not matter at all. CPU0CPU1 now0 = rdtsc_ordered(); ... tk->cycle_last = now0; gtod->seq++; gtod->cycle_last = tk->cycle_last; ... gtod->seq++; seq_begin(gtod->seq); now1 = rdtsc_ordered(); So if the TSC on CPU1 is slightly behind the TSC on CPU0 then now1 can be smaller than cycle_last. The TSC sync stuff does not catch the small delta for unknown raisins. I'll go and find that machine and test that again. Thanks, tglx ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [patch 09/11] x86/vdso: Simplify the invalid vclock case
On Tue, Sep 18, 2018 at 03:23:08PM +0200, Thomas Gleixner wrote: > On Tue, 18 Sep 2018, Peter Zijlstra wrote: > > On Tue, Sep 18, 2018 at 12:41:57PM +0200, Thomas Gleixner wrote: > > > I still have one of the machines which is affected by this. > > > > Are we sure this isn't a load vs rdtsc reorder? Because if I look at the > > current code: > > The load order of last vs. rdtsc does not matter at all. > > CPU0 CPU1 > > > now0 = rdtsc_ordered(); > ... > tk->cycle_last = now0; > > gtod->seq++; > gtod->cycle_last = tk->cycle_last; > ... > gtod->seq++; > seq_begin(gtod->seq); > now1 = rdtsc_ordered(); > > So if the TSC on CPU1 is slightly behind the TSC on CPU0 then now1 can be > smaller than cycle_last. The TSC sync stuff does not catch the small delta > for unknown raisins. I'll go and find that machine and test that again. Yeah, somehow I forgot about rseq.. maybe I should go sleep or something. ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH] staging: erofs: initialize `pblk' with 0 first in `z_erofs_map_blocks_iter'
This commit message helps to understand why `pblk' is assigned with 0 here. [ Greg reported a warning raised by gcc. ] In file included from drivers/staging/erofs/unzip_vle.h:16:0, from drivers/staging/erofs/unzip_vle.c:13: drivers/staging/erofs/unzip_vle.c: In function ‘z_erofs_map_blocks_iter’: drivers/staging/erofs/internal.h:303:34: warning: ‘pblk’ may be used uninitialized in this function [-Wmaybe-uninitialized] #define blknr_to_addr(nr) ((erofs_off_t)(nr) * EROFS_BLKSIZ) ^ drivers/staging/erofs/unzip_vle.c:1574:20: note: ‘pblk’ was declared here erofs_blk_t mblk, pblk; ^~~~ Actually, it is a false-positive warning when looking into that. [1] Just initialize the variable to keep gcc from printing foolish warnings. [1] https://lists.ozlabs.org/pipermail/linux-erofs/2018-September/000637.html Reported-by: Greg Kroah-Hartman Thanks-to: Dan Carpenter Signed-off-by: Gao Xiang --- drivers/staging/erofs/unzip_vle.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/staging/erofs/unzip_vle.c b/drivers/staging/erofs/unzip_vle.c index ad3b7bb..3ff8adf 100644 --- a/drivers/staging/erofs/unzip_vle.c +++ b/drivers/staging/erofs/unzip_vle.c @@ -1571,7 +1571,7 @@ int z_erofs_map_blocks_iter(struct inode *inode, unsigned int lcn; u32 ofs_rem; - erofs_blk_t mblk, pblk; + erofs_blk_t mblk, pblk = 0; struct page *mpage = *mpage_ret; struct z_erofs_vle_decompressed_index *di; unsigned int cluster_type, logical_cluster_ofs; -- 1.9.1 ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH v2 1/6] staging: erofs: code cleanup for erofs_kmalloc()
On 2018/9/18 18:41, Chao Yu wrote: > On 2018/9/17 23:34, Chengguang Xu wrote: >> Define a dummy function of time_to_inject()/erofs_show_injection_info(), >> so that we don't have to check macro CONFIG_EROFS_FAULT_INJECTION in >> calling place. >> >> Signed-off-by: Chengguang Xu > > Reviewed-by: Chao Yu > Reviewed-by: Gao Xiang Thanks, Gao Xiang > Thanks, > ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH v2 2/6] staging: erofs: code cleanup for option parsing of fault_injection
Hi Chengguang,
On 2018/9/18 18:47, cgxu519 wrote:
> On 09/18/2018 03:07 PM, Gao Xiang wrote:
>> Hi Chengguang,
>>
>> On 2018/9/17 23:34, Chengguang Xu wrote:
>>> Define a dummpy function of erofs_build_fault_attr() when macro
>>> CONFIG_EROFS_FAULT_INJECTION is disabled, so that we don't have to
>>> check the macro in calling place. Based on above adjustment,
>>> do proper code cleanup for option parsing of fault_injection.
>>>
>>> Signed-off-by: Chengguang Xu
>>> ---
>>> drivers/staging/erofs/super.c | 33 -
>>> 1 file changed, 20 insertions(+), 13 deletions(-)
>>>
>>> diff --git a/drivers/staging/erofs/super.c b/drivers/staging/erofs/super.c
>>> index 9e421536cbdf..7ce2fd3d49f3 100644
>>> --- a/drivers/staging/erofs/super.c
>>> +++ b/drivers/staging/erofs/super.c
>>> @@ -145,10 +145,14 @@ char *erofs_fault_name[FAULT_MAX] = {
>>> [FAULT_KMALLOC] = "kmalloc",
>>> };
>>> -static void erofs_build_fault_attr(struct erofs_sb_info *sbi,
>>> - unsigned int rate)
>>> +static int erofs_build_fault_attr(struct erofs_sb_info *sbi,
>>> + substring_t *args)
>>> {
>>> struct erofs_fault_info *ffi = &sbi->fault_info;
>>> + int rate = 0;
>>> +
>>> + if (args->from && match_int(args, &rate))
>>> + return -EINVAL;
>>> if (rate) {
>>> atomic_set(&ffi->inject_ops, 0);
>>> @@ -157,6 +161,15 @@ static void erofs_build_fault_attr(struct
>>> erofs_sb_info *sbi,
>>> } else {
>>> memset(ffi, 0, sizeof(struct erofs_fault_info));
>>> }
>>> +
>>> + set_opt(sbi, FAILt_INJECTION);
>> drivers/staging/erofs/super.c: In function ‘__erofs_build_fault_attr’:
>> drivers/staging/erofs/internal.h:176:51: error:
>> ‘EROFS_MOUNT_FAILt_INJECTION’ undeclared (first use in this function)
>> #define set_opt(sbi, option) ((sbi)->mount_opt |= EROFS_MOUNT_##option)
>> ^
>> drivers/staging/erofs/super.c:166:2: note: in expansion of macro ‘set_opt’
>> set_opt(sbi, FAILt_INJECTION);
>> ^
>> drivers/staging/erofs/internal.h:176:51: note: each undeclared identifier is
>> reported only once for each function it appears in
>> #define set_opt(sbi, option) ((sbi)->mount_opt |= EROFS_MOUNT_##option)
>> ^
>> drivers/staging/erofs/super.c:166:2: note: in expansion of macro ‘set_opt’
>> set_opt(sbi, FAILt_INJECTION);
>> ^
>
> Hi Xiang,
>
> I'm really sorry for that, I'm curious how it passed my building test.
> I deleted all existing config and binary files and tested with/without
> INJECTION config this time.
I have no idea either...
No worry about that, just resend your fixed patch.
Thanks,
Gao Xiang
>
>
> Thanks,
> Chengguang
>
>
>
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH] staging: erofs: initialize `pblk' with 0 first in `z_erofs_map_blocks_iter'
On Tue, Sep 18, 2018 at 09:44:36PM +0800, Gao Xiang wrote: > This commit message helps to understand why `pblk' is assigned with 0 here. > > [ Greg reported a warning raised by gcc. ] > In file included from drivers/staging/erofs/unzip_vle.h:16:0, > from drivers/staging/erofs/unzip_vle.c:13: > drivers/staging/erofs/unzip_vle.c: In function ‘z_erofs_map_blocks_iter’: > drivers/staging/erofs/internal.h:303:34: warning: ‘pblk’ may be used > uninitialized in this function [-Wmaybe-uninitialized] > #define blknr_to_addr(nr) ((erofs_off_t)(nr) * EROFS_BLKSIZ) > ^ > drivers/staging/erofs/unzip_vle.c:1574:20: note: ‘pblk’ was declared here > erofs_blk_t mblk, pblk; > ^~~~ > > Actually, it is a false-positive warning when looking into that. [1] > Just initialize the variable to keep gcc from printing foolish warnings. > > [1] https://lists.ozlabs.org/pipermail/linux-erofs/2018-September/000637.html > > Reported-by: Greg Kroah-Hartman > Thanks-to: Dan Carpenter > Signed-off-by: Gao Xiang > --- > drivers/staging/erofs/unzip_vle.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) Please roll this into the original patch. I have dropped that whole series from my tree, it is no longer present and needs to be resent in order for me to be able to accept any of these. thanks, greg k-h ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH] staging: erofs: initialize `pblk' with 0 first in `z_erofs_map_blocks_iter'
Hi Greg, On 2018/9/18 21:57, Greg Kroah-Hartman wrote: > On Tue, Sep 18, 2018 at 09:44:36PM +0800, Gao Xiang wrote: >> This commit message helps to understand why `pblk' is assigned with 0 here. >> >> [ Greg reported a warning raised by gcc. ] >> In file included from drivers/staging/erofs/unzip_vle.h:16:0, >> from drivers/staging/erofs/unzip_vle.c:13: >> drivers/staging/erofs/unzip_vle.c: In function ‘z_erofs_map_blocks_iter’: >> drivers/staging/erofs/internal.h:303:34: warning: ‘pblk’ may be used >> uninitialized in this function [-Wmaybe-uninitialized] >> #define blknr_to_addr(nr) ((erofs_off_t)(nr) * EROFS_BLKSIZ) >> ^ >> drivers/staging/erofs/unzip_vle.c:1574:20: note: ‘pblk’ was declared here >> erofs_blk_t mblk, pblk; >> ^~~~ >> >> Actually, it is a false-positive warning when looking into that. [1] >> Just initialize the variable to keep gcc from printing foolish warnings. >> >> [1] https://lists.ozlabs.org/pipermail/linux-erofs/2018-September/000637.html >> >> Reported-by: Greg Kroah-Hartman >> Thanks-to: Dan Carpenter >> Signed-off-by: Gao Xiang >> --- >> drivers/staging/erofs/unzip_vle.c | 2 +- >> 1 file changed, 1 insertion(+), 1 deletion(-) > > Please roll this into the original patch. I have dropped that whole > series from my tree, it is no longer present and needs to be resent in > order for me to be able to accept any of these. OK, will send. it's my honor. Thanks, Gao Xiang > > thanks, > > greg k-h > ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [patch 09/11] x86/vdso: Simplify the invalid vclock case
> On Sep 18, 2018, at 12:52 AM, Thomas Gleixner wrote: > >> On Mon, 17 Sep 2018, John Stultz wrote: >>> On Mon, Sep 17, 2018 at 12:25 PM, Andy Lutomirski wrote: >>> Also, I'm not entirely convinced that this "last" thing is needed at >>> all. John, what's the scenario under which we need it? >> >> So my memory is probably a bit foggy, but I recall that as we >> accelerated gettimeofday, we found that even on systems that claimed >> to have synced TSCs, they were actually just slightly out of sync. >> Enough that right after cycles_last had been updated, a read on >> another cpu could come in just behind cycles_last, resulting in a >> negative interval causing lots of havoc. >> >> So the sanity check is needed to avoid that case. > > Your memory serves you right. That's indeed observable on CPUs which > lack TSC_ADJUST. > > @Andy: Welcome to the wonderful world of TSC. > Do we do better if we use signed arithmetic for the whole calculation? Then a small backwards movement would result in a small backwards result. Or we could offset everything so that we’d have to go back several hundred ms before we cross zero. ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH v2 1/8] staging: erofs: fix a missing endian conversion
This patch fixes a missing endian conversion in
vle_get_logical_extent_head.
Reviewed-by: Chao Yu
Signed-off-by: Gao Xiang
---
drivers/staging/erofs/unzip_vle.c | 8 +---
1 file changed, 5 insertions(+), 3 deletions(-)
diff --git a/drivers/staging/erofs/unzip_vle.c
b/drivers/staging/erofs/unzip_vle.c
index 21874b7..d16e3dc 100644
--- a/drivers/staging/erofs/unzip_vle.c
+++ b/drivers/staging/erofs/unzip_vle.c
@@ -1488,6 +1488,7 @@ static erofs_off_t vle_get_logical_extent_head(
struct super_block *const sb = inode->i_sb;
const unsigned int clusterbits = EROFS_SB(sb)->clusterbits;
const unsigned int clustersize = 1 << clusterbits;
+ unsigned int delta0;
if (page->index != blkaddr) {
kunmap_atomic(*kaddr_iter);
@@ -1502,12 +1503,13 @@ static erofs_off_t vle_get_logical_extent_head(
di = *kaddr_iter + vle_extent_blkoff(inode, lcn);
switch (vle_cluster_type(di)) {
case Z_EROFS_VLE_CLUSTER_TYPE_NONHEAD:
- BUG_ON(!di->di_u.delta[0]);
- BUG_ON(lcn < di->di_u.delta[0]);
+ delta0 = le16_to_cpu(di->di_u.delta[0]);
+ DBG_BUGON(!delta0);
+ DBG_BUGON(lcn < delta0);
ofs = vle_get_logical_extent_head(inode,
page_iter, kaddr_iter,
- lcn - di->di_u.delta[0], pcn, flags);
+ lcn - delta0, pcn, flags);
break;
case Z_EROFS_VLE_CLUSTER_TYPE_PLAIN:
*flags ^= EROFS_MAP_ZIPPED;
--
1.9.1
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH v2 2/8] staging: erofs: clean up z_erofs_map_blocks_iter
This patch mainly introduces `vle_map_blocks_iter_ctx' to clean up
z_erofs_map_blocks_iter and vle_get_logical_extent_head.
It changes the return value of `vle_get_logical_extent_head' to int
for the later error handing. In addition, it also renames `pcn' to
`pblk' since only `pblk' exists in erofs compression ondisk format.
Reviewed-by: Chao Yu
Signed-off-by: Gao Xiang
---
drivers/staging/erofs/unzip_vle.c | 140 +-
1 file changed, 78 insertions(+), 62 deletions(-)
diff --git a/drivers/staging/erofs/unzip_vle.c
b/drivers/staging/erofs/unzip_vle.c
index d16e3dc..1e89ff6 100644
--- a/drivers/staging/erofs/unzip_vle.c
+++ b/drivers/staging/erofs/unzip_vle.c
@@ -1410,6 +1410,13 @@ static int z_erofs_vle_normalaccess_readpages(
.readpages = z_erofs_vle_normalaccess_readpages,
};
+/*
+ * Variable-sized Logical Extent (Fixed Physical Cluster) Compression Mode
+ * ---
+ * VLE compression mode attempts to compress a number of logical data into
+ * a physical cluster with a fixed size.
+ * VLE compression mode uses "struct z_erofs_vle_decompressed_index".
+ */
#define __vle_cluster_advise(x, bit, bits) \
((le16_to_cpu(x) >> (bit)) & ((1 << (bits)) - 1))
@@ -1465,90 +1472,96 @@ static int z_erofs_vle_normalaccess_readpages(
return erofs_blkoff(iloc(sbi, vi->nid) + ofs);
}
-/*
- * Variable-sized Logical Extent (Fixed Physical Cluster) Compression Mode
- * ---
- * VLE compression mode attempts to compress a number of logical data into
- * a physical cluster with a fixed size.
- * VLE compression mode uses "struct z_erofs_vle_decompressed_index".
- */
-static erofs_off_t vle_get_logical_extent_head(
- struct inode *inode,
- struct page **page_iter,
- void **kaddr_iter,
- unsigned int lcn, /* logical cluster number */
- erofs_blk_t *pcn,
- unsigned int *flags)
+struct vle_map_blocks_iter_ctx {
+ struct inode *inode;
+ struct super_block *sb;
+ unsigned int clusterbits;
+
+ struct page **mpage_ret;
+ void **kaddr_ret;
+};
+
+static int
+vle_get_logical_extent_head(const struct vle_map_blocks_iter_ctx *ctx,
+ unsigned int lcn, /* logical cluster number */
+ unsigned long long *ofs,
+ erofs_blk_t *pblk,
+ unsigned int *flags)
{
- /* for extent meta */
- struct page *page = *page_iter;
- erofs_blk_t blkaddr = vle_extent_blkaddr(inode, lcn);
+ const unsigned int clustersize = 1 << ctx->clusterbits;
+ const erofs_blk_t mblk = vle_extent_blkaddr(ctx->inode, lcn);
+ struct page *mpage = *ctx->mpage_ret; /* extent metapage */
+
struct z_erofs_vle_decompressed_index *di;
- unsigned long long ofs;
- struct super_block *const sb = inode->i_sb;
- const unsigned int clusterbits = EROFS_SB(sb)->clusterbits;
- const unsigned int clustersize = 1 << clusterbits;
- unsigned int delta0;
-
- if (page->index != blkaddr) {
- kunmap_atomic(*kaddr_iter);
- unlock_page(page);
- put_page(page);
+ unsigned int cluster_type, delta0;
- page = erofs_get_meta_page_nofail(sb, blkaddr, false);
- *page_iter = page;
- *kaddr_iter = kmap_atomic(page);
+ if (mpage->index != mblk) {
+ kunmap_atomic(*ctx->kaddr_ret);
+ unlock_page(mpage);
+ put_page(mpage);
+
+ mpage = erofs_get_meta_page_nofail(ctx->sb, mblk, false);
+ *ctx->mpage_ret = mpage;
+ *ctx->kaddr_ret = kmap_atomic(mpage);
}
- di = *kaddr_iter + vle_extent_blkoff(inode, lcn);
- switch (vle_cluster_type(di)) {
+ di = *ctx->kaddr_ret + vle_extent_blkoff(ctx->inode, lcn);
+
+ cluster_type = vle_cluster_type(di);
+ switch (cluster_type) {
case Z_EROFS_VLE_CLUSTER_TYPE_NONHEAD:
delta0 = le16_to_cpu(di->di_u.delta[0]);
DBG_BUGON(!delta0);
DBG_BUGON(lcn < delta0);
- ofs = vle_get_logical_extent_head(inode,
- page_iter, kaddr_iter,
- lcn - delta0, pcn, flags);
- break;
+ return vle_get_logical_extent_head(ctx,
+ lcn - delta0, ofs, pblk, flags);
case Z_EROFS_VLE_CLUSTER_TYPE_PLAIN:
*flags ^= EROFS_MAP_ZIPPED;
+ /* fallthrough */
case Z_EROFS_VLE_CLUSTER_TYPE_HEAD:
/* clustersize should be a power of two */
- ofs = ((u64)lcn << clusterbits) +
+ *ofs = ((u64)lcn << ctx->clusterbits) +
(le16_to_cpu(di->di_clusterofs) & (clustersize - 1));
- *pcn = le32_to_cpu(di->di_u.blkaddr);
+ *pblk = le32_to_cpu(di->di_u.blkaddr);
break;
default:
[PATCH v2 0/8] staging: erofs: error handing and more tracepoints
change log v2: - fix a gcc warning reported by Greg for the patch staging: erofs: clean up z_erofs_map_blocks_iter In order to avoid conflicts with cleanup patches, Chao and I think it is better to send reviewed preview patches in the erofs mailing list to the community in time. So here is reviewed & tested patches right now, which clean up and enhance the error handing and add some tracepoints for decompression. Note that in this patchset, bare use of 'unsigned' and NULL comparison are also fixed compared with the preview patches according to the previous discussion in the staging mailing list. Thanks, Gao Xiang Chen Gong (2): staging: erofs: add trace points for reading zipped data staging: erofs: replace BUG_ON with DBG_BUGON in data.c Gao Xiang (6): staging: erofs: fix a missing endian conversion staging: erofs: clean up z_erofs_map_blocks_iter staging: erofs: complete error handing of z_erofs_map_blocks_iter staging: erofs: fix a bug when appling cache strategy staging: erofs: complete error handing of z_erofs_do_read_page staging: erofs: avoid magic constants when initializing clusterbits drivers/staging/erofs/data.c | 31 ++-- drivers/staging/erofs/include/trace/events/erofs.h | 20 ++- drivers/staging/erofs/super.c | 5 +- drivers/staging/erofs/unzip_vle.c | 196 + 4 files changed, 163 insertions(+), 89 deletions(-) -- 1.9.1 ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH v2 3/8] staging: erofs: complete error handing of z_erofs_map_blocks_iter
This patch completes error handing of z_erofs_map_blocks_iter
and vle_get_logical_extent_head, including no memory and
io error cases.
Reviewed-by: Chao Yu
Signed-off-by: Gao Xiang
---
drivers/staging/erofs/unzip_vle.c | 35 ++-
1 file changed, 26 insertions(+), 9 deletions(-)
diff --git a/drivers/staging/erofs/unzip_vle.c
b/drivers/staging/erofs/unzip_vle.c
index 1e89ff6..94c35b8 100644
--- a/drivers/staging/erofs/unzip_vle.c
+++ b/drivers/staging/erofs/unzip_vle.c
@@ -1500,7 +1500,11 @@ struct vle_map_blocks_iter_ctx {
unlock_page(mpage);
put_page(mpage);
- mpage = erofs_get_meta_page_nofail(ctx->sb, mblk, false);
+ mpage = erofs_get_meta_page(ctx->sb, mblk, false);
+ if (IS_ERR(mpage)) {
+ *ctx->mpage_ret = NULL;
+ return PTR_ERR(mpage);
+ }
*ctx->mpage_ret = mpage;
*ctx->kaddr_ret = kmap_atomic(mpage);
}
@@ -1511,9 +1515,12 @@ struct vle_map_blocks_iter_ctx {
switch (cluster_type) {
case Z_EROFS_VLE_CLUSTER_TYPE_NONHEAD:
delta0 = le16_to_cpu(di->di_u.delta[0]);
- DBG_BUGON(!delta0);
- DBG_BUGON(lcn < delta0);
-
+ if (unlikely(!delta0 || delta0 > lcn)) {
+ errln("invalid NONHEAD dl0 %u at lcn %u of nid %llu",
+ delta0, lcn, EROFS_V(ctx->inode)->nid);
+ DBG_BUGON(1);
+ return -EIO;
+ }
return vle_get_logical_extent_head(ctx,
lcn - delta0, ofs, pblk, flags);
case Z_EROFS_VLE_CLUSTER_TYPE_PLAIN:
@@ -1526,7 +1533,10 @@ struct vle_map_blocks_iter_ctx {
*pblk = le32_to_cpu(di->di_u.blkaddr);
break;
default:
- BUG_ON(1);
+ errln("unknown cluster type %u at lcn %u of nid %llu",
+ cluster_type, lcn, EROFS_V(ctx->inode)->nid);
+ DBG_BUGON(1);
+ return -EIO;
}
return 0;
}
@@ -1583,7 +1593,11 @@ int z_erofs_map_blocks_iter(struct inode *inode,
if (mpage != NULL)
put_page(mpage);
- mpage = erofs_get_meta_page_nofail(ctx.sb, mblk, false);
+ mpage = erofs_get_meta_page(ctx.sb, mblk, false);
+ if (IS_ERR(mpage)) {
+ err = PTR_ERR(mpage);
+ goto out;
+ }
*mpage_ret = mpage;
} else {
lock_page(mpage);
@@ -1646,8 +1660,11 @@ int z_erofs_map_blocks_iter(struct inode *inode,
&pblk, &map->m_flags);
mpage = *mpage_ret;
- if (unlikely(err))
- goto unmap_out;
+ if (unlikely(err)) {
+ if (mpage)
+ goto unmap_out;
+ goto out;
+ }
break;
default:
errln("unknown cluster type %u at offset %llu of nid %llu",
@@ -1671,7 +1688,7 @@ int z_erofs_map_blocks_iter(struct inode *inode,
map->m_llen, map->m_plen, map->m_flags);
/* aggressively BUG_ON iff CONFIG_EROFS_FS_DEBUG is on */
- DBG_BUGON(err < 0);
+ DBG_BUGON(err < 0 && err != -ENOMEM);
return err;
}
--
1.9.1
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH v2 4/8] staging: erofs: fix a bug when appling cache strategy
As described in Kconfig, the last compressed pack should be cached
for further reading for either `EROFS_FS_ZIP_CACHE_UNIPOLAR' or
`EROFS_FS_ZIP_CACHE_BIPOLAR' by design.
However, there is a bug in z_erofs_do_read_page, it will
switch `initial' to `false' at the very beginning before it decides
to cache the last compressed pack.
caching strategy should work properly after appling this patch.
Reviewed-by: Chao Yu
Signed-off-by: Gao Xiang
---
drivers/staging/erofs/unzip_vle.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/staging/erofs/unzip_vle.c
b/drivers/staging/erofs/unzip_vle.c
index 94c35b8..3296538 100644
--- a/drivers/staging/erofs/unzip_vle.c
+++ b/drivers/staging/erofs/unzip_vle.c
@@ -629,7 +629,7 @@ static int z_erofs_do_read_page(struct z_erofs_vle_frontend
*fe,
/* go ahead the next map_blocks */
debugln("%s: [out-of-range] pos %llu", __func__, offset + cur);
- if (!z_erofs_vle_work_iter_end(builder))
+ if (z_erofs_vle_work_iter_end(builder))
fe->initial = false;
map->m_la = offset + cur;
--
1.9.1
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH v2 6/8] staging: erofs: avoid magic constants when initializing clusterbits
Currently erofs only supports clustersize == blocksize.
and clustersize == 2^n * blocksize will be supported in the future.
Reviewed-by: Chao Yu
Signed-off-by: Gao Xiang
---
drivers/staging/erofs/super.c | 5 +++--
1 file changed, 3 insertions(+), 2 deletions(-)
diff --git a/drivers/staging/erofs/super.c b/drivers/staging/erofs/super.c
index 2109b03..9e42153 100644
--- a/drivers/staging/erofs/super.c
+++ b/drivers/staging/erofs/super.c
@@ -116,9 +116,10 @@ static int superblock_read(struct super_block *sb)
#endif
sbi->islotbits = ffs(sizeof(struct erofs_inode_v1)) - 1;
#ifdef CONFIG_EROFS_FS_ZIP
- sbi->clusterbits = 12;
+ /* TODO: clusterbits should be related to inode */
+ sbi->clusterbits = blkszbits;
- if (1 << (sbi->clusterbits - 12) > Z_EROFS_CLUSTER_MAX_PAGES)
+ if (1 << (sbi->clusterbits - PAGE_SHIFT) > Z_EROFS_CLUSTER_MAX_PAGES)
errln("clusterbits %u is not supported on this kernel",
sbi->clusterbits);
#endif
--
1.9.1
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH v2 5/8] staging: erofs: complete error handing of z_erofs_do_read_page
This patch completes error handing code of z_erofs_do_read_page.
PG_error will be set when some read error happens, therefore
z_erofs_onlinepage_endio will unlock this page without setting
PG_uptodate.
Reviewed-by: Chao Yu
Signed-off-by: Gao Xiang
---
drivers/staging/erofs/unzip_vle.c | 20 +++-
1 file changed, 11 insertions(+), 9 deletions(-)
diff --git a/drivers/staging/erofs/unzip_vle.c
b/drivers/staging/erofs/unzip_vle.c
index 3296538..08c424c 100644
--- a/drivers/staging/erofs/unzip_vle.c
+++ b/drivers/staging/erofs/unzip_vle.c
@@ -611,7 +611,7 @@ static int z_erofs_do_read_page(struct z_erofs_vle_frontend
*fe,
enum z_erofs_page_type page_type;
unsigned int cur, end, spiltted, index;
- int err;
+ int err = 0;
/* register locked file pages as online pages in pack */
z_erofs_onlinepage_init(page);
@@ -638,12 +638,11 @@ static int z_erofs_do_read_page(struct
z_erofs_vle_frontend *fe,
if (unlikely(err))
goto err_out;
- /* deal with hole (FIXME! broken now) */
if (unlikely(!(map->m_flags & EROFS_MAP_MAPPED)))
goto hitted;
DBG_BUGON(map->m_plen != 1 << sbi->clusterbits);
- BUG_ON(erofs_blkoff(map->m_pa));
+ DBG_BUGON(erofs_blkoff(map->m_pa));
err = z_erofs_vle_work_iter_begin(builder, sb, map, &fe->owned_head);
if (unlikely(err))
@@ -688,7 +687,7 @@ static int z_erofs_do_read_page(struct z_erofs_vle_frontend
*fe,
err = z_erofs_vle_work_add_page(builder,
newpage, Z_EROFS_PAGE_TYPE_EXCLUSIVE);
- if (!err)
+ if (likely(!err))
goto retry;
}
@@ -699,9 +698,10 @@ static int z_erofs_do_read_page(struct
z_erofs_vle_frontend *fe,
/* FIXME! avoid the last relundant fixup & endio */
z_erofs_onlinepage_fixup(page, index, true);
- ++spiltted;
- /* also update nr_pages and increase queued_pages */
+ /* bump up the number of spiltted parts of a page */
+ ++spiltted;
+ /* also update nr_pages */
work->nr_pages = max_t(pgoff_t, work->nr_pages, index + 1);
next_part:
/* can be used for verification */
@@ -711,16 +711,18 @@ static int z_erofs_do_read_page(struct
z_erofs_vle_frontend *fe,
if (end > 0)
goto repeat;
+out:
/* FIXME! avoid the last relundant fixup & endio */
z_erofs_onlinepage_endio(page);
debugln("%s, finish page: %pK spiltted: %u map->m_llen %llu",
__func__, page, spiltted, map->m_llen);
- return 0;
+ return err;
+ /* if some error occurred while processing this page */
err_out:
- /* TODO: the missing error handing cases */
- return err;
+ SetPageError(page);
+ goto out;
}
static void z_erofs_vle_unzip_kickoff(void *ptr, int bios)
--
1.9.1
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH v2 7/8] staging: erofs: add trace points for reading zipped data
From: Chen Gong
This patch adds trace points for reading zipped data.
Signed-off-by: Chen Gong
Reviewed-by: Chao Yu
Reviewed-by: Gao Xiang
Signed-off-by: Gao Xiang
---
drivers/staging/erofs/include/trace/events/erofs.h | 20 ++--
drivers/staging/erofs/unzip_vle.c | 11 +++
2 files changed, 29 insertions(+), 2 deletions(-)
diff --git a/drivers/staging/erofs/include/trace/events/erofs.h
b/drivers/staging/erofs/include/trace/events/erofs.h
index 5aead93..660c92f 100644
--- a/drivers/staging/erofs/include/trace/events/erofs.h
+++ b/drivers/staging/erofs/include/trace/events/erofs.h
@@ -162,7 +162,8 @@
TP_printk("dev = (%d,%d), nid = %llu, la %llu llen %llu flags %s",
show_dev_nid(__entry),
- __entry->la, __entry->llen, show_map_flags(__entry->flags))
+ __entry->la, __entry->llen,
+ __entry->flags ? show_map_flags(__entry->flags) : "NULL")
);
DEFINE_EVENT(erofs__map_blocks_enter, erofs_map_blocks_flatmode_enter,
@@ -172,6 +173,13 @@
TP_ARGS(inode, map, flags)
);
+DEFINE_EVENT(erofs__map_blocks_enter, z_erofs_map_blocks_iter_enter,
+ TP_PROTO(struct inode *inode, struct erofs_map_blocks *map,
+unsigned int flags),
+
+ TP_ARGS(inode, map, flags)
+);
+
DECLARE_EVENT_CLASS(erofs__map_blocks_exit,
TP_PROTO(struct inode *inode, struct erofs_map_blocks *map,
unsigned int flags, int ret),
@@ -204,7 +212,8 @@
TP_printk("dev = (%d,%d), nid = %llu, flags %s "
"la %llu pa %llu llen %llu plen %llu mflags %s ret %d",
- show_dev_nid(__entry), show_map_flags(__entry->flags),
+ show_dev_nid(__entry),
+ __entry->flags ? show_map_flags(__entry->flags) : "NULL",
__entry->la, __entry->pa, __entry->llen, __entry->plen,
show_mflags(__entry->mflags), __entry->ret)
);
@@ -216,6 +225,13 @@
TP_ARGS(inode, map, flags, ret)
);
+DEFINE_EVENT(erofs__map_blocks_exit, z_erofs_map_blocks_iter_exit,
+ TP_PROTO(struct inode *inode, struct erofs_map_blocks *map,
+unsigned int flags, int ret),
+
+ TP_ARGS(inode, map, flags, ret)
+);
+
TRACE_EVENT(erofs_destroy_inode,
TP_PROTO(struct inode *inode),
diff --git a/drivers/staging/erofs/unzip_vle.c
b/drivers/staging/erofs/unzip_vle.c
index 08c424c..2b9b313 100644
--- a/drivers/staging/erofs/unzip_vle.c
+++ b/drivers/staging/erofs/unzip_vle.c
@@ -13,6 +13,8 @@
#include "unzip_vle.h"
#include
+#include
+
static struct workqueue_struct *z_erofs_workqueue __read_mostly;
static struct kmem_cache *z_erofs_workgroup_cachep __read_mostly;
@@ -613,6 +615,8 @@ static int z_erofs_do_read_page(struct z_erofs_vle_frontend
*fe,
unsigned int cur, end, spiltted, index;
int err = 0;
+ trace_erofs_readpage(page, false);
+
/* register locked file pages as online pages in pack */
z_erofs_onlinepage_init(page);
@@ -1348,6 +1352,9 @@ static inline int __z_erofs_vle_normalaccess_readpages(
struct page *head = NULL;
LIST_HEAD(pagepool);
+ trace_erofs_readpages(mapping->host, lru_to_page(pages),
+ nr_pages, false);
+
#if (EROFS_FS_ZIP_CACHE_LVL >= 2)
f.cachedzone_la = (erofs_off_t)lru_to_page(pages)->index << PAGE_SHIFT;
#endif
@@ -1571,6 +1578,8 @@ int z_erofs_map_blocks_iter(struct inode *inode,
unsigned int cluster_type, logical_cluster_ofs;
int err = 0;
+ trace_z_erofs_map_blocks_iter_enter(inode, map, flags);
+
/* when trying to read beyond EOF, leave it unmapped */
if (unlikely(map->m_la >= inode->i_size)) {
DBG_BUGON(!initial);
@@ -1689,6 +1698,8 @@ int z_erofs_map_blocks_iter(struct inode *inode,
__func__, map->m_la, map->m_pa,
map->m_llen, map->m_plen, map->m_flags);
+ trace_z_erofs_map_blocks_iter_exit(inode, map, flags, err);
+
/* aggressively BUG_ON iff CONFIG_EROFS_FS_DEBUG is on */
DBG_BUGON(err < 0 && err != -ENOMEM);
return err;
--
1.9.1
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH v2 8/8] staging: erofs: replace BUG_ON with DBG_BUGON in data.c
From: Chen Gong
This patch replace BUG_ON with DBG_BUGON in data.c, and add necessary
error handler.
Signed-off-by: Chen Gong
Reviewed-by: Gao Xiang
Reviewed-by: Chao Yu
Signed-off-by: Gao Xiang
---
drivers/staging/erofs/data.c | 31 ---
1 file changed, 20 insertions(+), 11 deletions(-)
diff --git a/drivers/staging/erofs/data.c b/drivers/staging/erofs/data.c
index e191610..6384f73 100644
--- a/drivers/staging/erofs/data.c
+++ b/drivers/staging/erofs/data.c
@@ -25,7 +25,7 @@ static inline void read_endio(struct bio *bio)
struct page *page = bvec->bv_page;
/* page is already locked */
- BUG_ON(PageUptodate(page));
+ DBG_BUGON(PageUptodate(page));
if (unlikely(err))
SetPageError(page);
@@ -110,12 +110,12 @@ static int erofs_map_blocks_flatmode(struct inode *inode,
struct erofs_map_blocks *map,
int flags)
{
+ int err = 0;
erofs_blk_t nblocks, lastblk;
u64 offset = map->m_la;
struct erofs_vnode *vi = EROFS_V(inode);
trace_erofs_map_blocks_flatmode_enter(inode, map, flags);
- BUG_ON(is_inode_layout_compression(inode));
nblocks = DIV_ROUND_UP(inode->i_size, PAGE_SIZE);
lastblk = nblocks - is_inode_layout_inline(inode);
@@ -142,18 +142,27 @@ static int erofs_map_blocks_flatmode(struct inode *inode,
map->m_plen = inode->i_size - offset;
/* inline data should locate in one meta block */
- BUG_ON(erofs_blkoff(map->m_pa) + map->m_plen > PAGE_SIZE);
+ if (erofs_blkoff(map->m_pa) + map->m_plen > PAGE_SIZE) {
+ DBG_BUGON(1);
+ err = -EIO;
+ goto err_out;
+ }
+
map->m_flags |= EROFS_MAP_META;
} else {
errln("internal error @ nid: %llu (size %llu), m_la 0x%llx",
vi->nid, inode->i_size, map->m_la);
- BUG();
+ DBG_BUGON(1);
+ err = -EIO;
+ goto err_out;
}
out:
map->m_llen = map->m_plen;
+
+err_out:
trace_erofs_map_blocks_flatmode_exit(inode, map, flags, 0);
- return 0;
+ return err;
}
#ifdef CONFIG_EROFS_FS_ZIP
@@ -209,7 +218,7 @@ static inline struct bio *erofs_read_raw_page(
erofs_off_t current_block = (erofs_off_t)page->index;
int err;
- BUG_ON(!nblocks);
+ DBG_BUGON(!nblocks);
if (PageUptodate(page)) {
err = 0;
@@ -252,7 +261,7 @@ static inline struct bio *erofs_read_raw_page(
}
/* for RAW access mode, m_plen must be equal to m_llen */
- BUG_ON(map.m_plen != map.m_llen);
+ DBG_BUGON(map.m_plen != map.m_llen);
blknr = erofs_blknr(map.m_pa);
blkoff = erofs_blkoff(map.m_pa);
@@ -262,7 +271,7 @@ static inline struct bio *erofs_read_raw_page(
void *vsrc, *vto;
struct page *ipage;
- BUG_ON(map.m_plen > PAGE_SIZE);
+ DBG_BUGON(map.m_plen > PAGE_SIZE);
ipage = erofs_get_meta_page(inode->i_sb, blknr, 0);
@@ -289,7 +298,7 @@ static inline struct bio *erofs_read_raw_page(
}
/* pa must be block-aligned for raw reading */
- BUG_ON(erofs_blkoff(map.m_pa) != 0);
+ DBG_BUGON(erofs_blkoff(map.m_pa));
/* max # of continuous pages */
if (nblocks > DIV_ROUND_UP(map.m_plen, PAGE_SIZE))
@@ -357,7 +366,7 @@ static int erofs_raw_access_readpage(struct file *file,
struct page *page)
if (IS_ERR(bio))
return PTR_ERR(bio);
- BUG_ON(bio != NULL);/* since we have only one bio -- must be NULL */
+ DBG_BUGON(bio); /* since we have only one bio -- must be NULL */
return 0;
}
@@ -395,7 +404,7 @@ static int erofs_raw_access_readpages(struct file *filp,
/* pages could still be locked */
put_page(page);
}
- BUG_ON(!list_empty(pages));
+ DBG_BUGON(!list_empty(pages));
/* the rare case (end in gaps) */
if (unlikely(bio != NULL))
--
1.9.1
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH v2 0/8] staging: erofs: error handing and more tracepoints
On Tue, Sep 18, 2018 at 10:25:32PM +0800, Gao Xiang wrote: > change log v2: > - fix a gcc warning reported by Greg for the patch > staging: erofs: clean up z_erofs_map_blocks_iter > > > In order to avoid conflicts with cleanup patches, Chao and I think > it is better to send reviewed preview patches in the erofs mailing list > to the community in time. > > So here is reviewed & tested patches right now, which clean up and > enhance the error handing and add some tracepoints for decompression. > > Note that in this patchset, bare use of 'unsigned' and NULL comparison are > also fixed compared with the preview patches according to the previous > discussion in the staging mailing list. Thanks for the resend, looks good. greg k-h ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH v2 0/8] staging: erofs: error handing and more tracepoints
Hi Greg, On 2018/9/18 22:36, Greg Kroah-Hartman wrote: > On Tue, Sep 18, 2018 at 10:25:32PM +0800, Gao Xiang wrote: >> change log v2: >> - fix a gcc warning reported by Greg for the patch >> staging: erofs: clean up z_erofs_map_blocks_iter >> >> >> In order to avoid conflicts with cleanup patches, Chao and I think >> it is better to send reviewed preview patches in the erofs mailing list >> to the community in time. >> >> So here is reviewed & tested patches right now, which clean up and >> enhance the error handing and add some tracepoints for decompression. >> >> Note that in this patchset, bare use of 'unsigned' and NULL comparison are >> also fixed compared with the preview patches according to the previous >> discussion in the staging mailing list. > > Thanks for the resend, looks good. Thanks for applying... p.s. sorry for just expression... I mean "with my pleasure", sorry about my english... Thanks, Gao Xiang > > greg k-h > ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH v3 1/4] staging: erofs: code cleanup for erofs_kmalloc()
Define a dummy function of time_to_inject()/erofs_show_injection_info(),
so that we don't have to check macro CONFIG_EROFS_FAULT_INJECTION in
calling place.
Signed-off-by: Chengguang Xu
Reviewed-by: Chao Yu
Reviewed-by: Gao Xiang
---
drivers/staging/erofs/internal.h | 13 ++---
1 file changed, 10 insertions(+), 3 deletions(-)
diff --git a/drivers/staging/erofs/internal.h b/drivers/staging/erofs/internal.h
index f20c6e9b7471..0011b9d505fd 100644
--- a/drivers/staging/erofs/internal.h
+++ b/drivers/staging/erofs/internal.h
@@ -42,12 +42,12 @@
#define DBG_BUGON(...) ((void)0)
#endif
-#ifdef CONFIG_EROFS_FAULT_INJECTION
enum {
FAULT_KMALLOC,
FAULT_MAX,
};
+#ifdef CONFIG_EROFS_FAULT_INJECTION
extern char *erofs_fault_name[FAULT_MAX];
#define IS_FAULT_SET(fi, type) ((fi)->inject_type & (1 << (type)))
@@ -143,17 +143,24 @@ static inline bool time_to_inject(struct erofs_sb_info
*sbi, int type)
}
return false;
}
+#else
+static inline bool time_to_inject(struct erofs_sb_info *sbi, int type)
+{
+ return false;
+}
+
+static inline void erofs_show_injection_info(int type)
+{
+}
#endif
static inline void *erofs_kmalloc(struct erofs_sb_info *sbi,
size_t size, gfp_t flags)
{
-#ifdef CONFIG_EROFS_FAULT_INJECTION
if (time_to_inject(sbi, FAULT_KMALLOC)) {
erofs_show_injection_info(FAULT_KMALLOC);
return NULL;
}
-#endif
return kmalloc(size, flags);
}
--
2.17.1
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH v3 4/4] staging: erofs: option validation in remount
Add option validation in remount. After this patch, remount
can change recognized options, and for unknown options remount
will fail and report error.
Signed-off-by: Chengguang Xu
Reviewed-by: Chao Yu
---
drivers/staging/erofs/super.c | 38 ---
1 file changed, 31 insertions(+), 7 deletions(-)
diff --git a/drivers/staging/erofs/super.c b/drivers/staging/erofs/super.c
index a091b93190e1..30b6b44dc6c4 100644
--- a/drivers/staging/erofs/super.c
+++ b/drivers/staging/erofs/super.c
@@ -145,14 +145,10 @@ char *erofs_fault_name[FAULT_MAX] = {
[FAULT_KMALLOC] = "kmalloc",
};
-static int erofs_build_fault_attr(struct erofs_sb_info *sbi,
- substring_t *args)
+static void __erofs_build_fault_attr(struct erofs_sb_info *sbi,
+ unsigned int rate)
{
struct erofs_fault_info *ffi = &sbi->fault_info;
- int rate = 0;
-
- if (args->from && match_int(args, &rate))
- return -EINVAL;
if (rate) {
atomic_set(&ffi->inject_ops, 0);
@@ -163,14 +159,29 @@ static int erofs_build_fault_attr(struct erofs_sb_info
*sbi,
}
set_opt(sbi, FAULT_INJECTION);
- return 0;
}
static unsigned int erofs_get_fault_rate(struct erofs_sb_info *sbi)
{
return sbi->fault_info.inject_rate;
}
+
+static int erofs_build_fault_attr(struct erofs_sb_info *sbi, substring_t *args)
+{
+ int rate = 0;
+
+ if (args->from && match_int(args, &rate))
+ return -EINVAL;
+
+ __erofs_build_fault_attr(sbi, rate);
+ return 0;
+}
#else
+static void __erofs_build_fault_attr(struct erofs_sb_info *sbi,
+ unsigned int rate)
+{
+}
+
static int erofs_build_fault_attr(struct erofs_sb_info *sbi,
substring_t *args)
{
@@ -644,10 +655,23 @@ static int erofs_show_options(struct seq_file *seq,
struct dentry *root)
static int erofs_remount(struct super_block *sb, int *flags, char *data)
{
+ struct erofs_sb_info *sbi = EROFS_SB(sb);
+ unsigned int org_mnt_opt = sbi->mount_opt;
+ unsigned int org_inject_rate = erofs_get_fault_rate(sbi);
+ int err;
+
BUG_ON(!sb_rdonly(sb));
+ err = parse_options(sb, data);
+ if (err)
+ goto out;
*flags |= MS_RDONLY;
return 0;
+out:
+ __erofs_build_fault_attr(sbi, org_inject_rate);
+ sbi->mount_opt = org_mnt_opt;
+
+ return err;
}
const struct super_operations erofs_sops = {
--
2.17.1
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH v3 2/4] staging: erofs: code cleanup for option parsing of fault_injection
Define a dummpy function of erofs_build_fault_attr() when macro
CONFIG_EROFS_FAULT_INJECTION is disabled, so that we don't have to
check the macro in calling place. Based on above adjustment,
do proper code cleanup for option parsing of fault_injection.
Signed-off-by: Chengguang Xu
Reviewed-by: Chao Yu
---
drivers/staging/erofs/super.c | 34 +-
1 file changed, 21 insertions(+), 13 deletions(-)
diff --git a/drivers/staging/erofs/super.c b/drivers/staging/erofs/super.c
index 9e421536cbdf..f496e0c1d04d 100644
--- a/drivers/staging/erofs/super.c
+++ b/drivers/staging/erofs/super.c
@@ -145,10 +145,14 @@ char *erofs_fault_name[FAULT_MAX] = {
[FAULT_KMALLOC] = "kmalloc",
};
-static void erofs_build_fault_attr(struct erofs_sb_info *sbi,
- unsigned int rate)
+static int erofs_build_fault_attr(struct erofs_sb_info *sbi,
+ substring_t *args)
{
struct erofs_fault_info *ffi = &sbi->fault_info;
+ int rate = 0;
+
+ if (args->from && match_int(args, &rate))
+ return -EINVAL;
if (rate) {
atomic_set(&ffi->inject_ops, 0);
@@ -157,6 +161,16 @@ static void erofs_build_fault_attr(struct erofs_sb_info
*sbi,
} else {
memset(ffi, 0, sizeof(struct erofs_fault_info));
}
+
+ set_opt(sbi, FAULT_INJECTION);
+ return 0;
+}
+#else
+static int erofs_build_fault_attr(struct erofs_sb_info *sbi,
+ substring_t *args)
+{
+ infoln("fault_injection options not supported");
+ return 0;
}
#endif
@@ -193,7 +207,7 @@ static int parse_options(struct super_block *sb, char
*options)
{
substring_t args[MAX_OPT_ARGS];
char *p;
- int arg = 0;
+ int err;
if (!options)
return 0;
@@ -238,18 +252,12 @@ static int parse_options(struct super_block *sb, char
*options)
infoln("noacl options not supported");
break;
#endif
-#ifdef CONFIG_EROFS_FAULT_INJECTION
case Opt_fault_injection:
- if (args->from && match_int(args, &arg))
- return -EINVAL;
- erofs_build_fault_attr(EROFS_SB(sb), arg);
- set_opt(EROFS_SB(sb), FAULT_INJECTION);
+ err = erofs_build_fault_attr(EROFS_SB(sb), args);
+ if (err)
+ return err;
break;
-#else
- case Opt_fault_injection:
- infoln("fault_injection options not supported");
- break;
-#endif
+
default:
errln("Unrecognized mount option \"%s\" "
"or missing value", p);
--
2.17.1
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH v3 0/4] staging: erofs: option validation for remount and some code cleanups
This patch set mainly does option validation for remount and at the same time does related code cleanups. Currently when we call fault injection related code we have to surround it with macro CONFIG_EROFS_FAULT_INJECTION in every calling place, after this patch set we don't have to do that, so the code looks clean and more understandable. v2->v3: - Fold related patches to one patch. - Fix building issue. v1->v2: Address Chao's comments: - Allow to set fault_injection=0. - Keep flag bit when setting fault_injection=0. - Show injection info in original place. - Rebase code on latest erofs branch in Chao's linux tree. - Fix building issue. Hi Greg, You may pick up rest 2-4 patches in this patch set if there are no more comments from Chao and Xiang. Thanks, Chengguang Xu (4): staging: erofs: code cleanup for erofs_kmalloc() staging: erofs: code cleanup for option parsing of fault_injection staging: erofs: code cleanup for erofs_show_options() staging: erofs: option validation in remount drivers/staging/erofs/internal.h | 13 -- drivers/staging/erofs/super.c| 73 +--- 2 files changed, 67 insertions(+), 19 deletions(-) -- 2.17.1 ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH v3 3/4] staging: erofs: code cleanup for erofs_show_options()
Add new helper erofs_get_fault_rate() to get fault rate instead of
directly getting it from sbi, so we can remove the macro check
surrounding it.
Signed-off-by: Chengguang Xu
Reviewed-by: Chao Yu
---
drivers/staging/erofs/super.c | 14 +++---
1 file changed, 11 insertions(+), 3 deletions(-)
diff --git a/drivers/staging/erofs/super.c b/drivers/staging/erofs/super.c
index f496e0c1d04d..a091b93190e1 100644
--- a/drivers/staging/erofs/super.c
+++ b/drivers/staging/erofs/super.c
@@ -165,6 +165,11 @@ static int erofs_build_fault_attr(struct erofs_sb_info
*sbi,
set_opt(sbi, FAULT_INJECTION);
return 0;
}
+
+static unsigned int erofs_get_fault_rate(struct erofs_sb_info *sbi)
+{
+ return sbi->fault_info.inject_rate;
+}
#else
static int erofs_build_fault_attr(struct erofs_sb_info *sbi,
substring_t *args)
@@ -172,6 +177,11 @@ static int erofs_build_fault_attr(struct erofs_sb_info
*sbi,
infoln("fault_injection options not supported");
return 0;
}
+
+static unsigned int erofs_get_fault_rate(struct erofs_sb_info *sbi)
+{
+ return 0;
+}
#endif
static void default_options(struct erofs_sb_info *sbi)
@@ -626,11 +636,9 @@ static int erofs_show_options(struct seq_file *seq, struct
dentry *root)
else
seq_puts(seq, ",noacl");
#endif
-#ifdef CONFIG_EROFS_FAULT_INJECTION
if (test_opt(sbi, FAULT_INJECTION))
seq_printf(seq, ",fault_injection=%u",
- sbi->fault_info.inject_rate);
-#endif
+ erofs_get_fault_rate(sbi));
return 0;
}
--
2.17.1
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [patch 09/11] x86/vdso: Simplify the invalid vclock case
On Tue, 18 Sep 2018, Thomas Gleixner wrote: > So if the TSC on CPU1 is slightly behind the TSC on CPU0 then now1 can be > smaller than cycle_last. The TSC sync stuff does not catch the small delta > for unknown raisins. I'll go and find that machine and test that again. Of course it does not trigger anymore. We accumulated code between the point in timekeeping_advance() where the TSC is read and the update of the VDSO data. I'll might have to get an 2.6ish kernel booted on that machine and try with that again. /me shudders Thanks, tglx ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH v3 2/4] staging: erofs: code cleanup for option parsing of fault_injection
Hi Chengguang,
On 2018/9/18 23:10, Chengguang Xu wrote:
> Define a dummpy function of erofs_build_fault_attr() when macro
> CONFIG_EROFS_FAULT_INJECTION is disabled, so that we don't have to
> check the macro in calling place. Based on above adjustment,
> do proper code cleanup for option parsing of fault_injection.
>
> Signed-off-by: Chengguang Xu
> Reviewed-by: Chao Yu
> ---
> drivers/staging/erofs/super.c | 34 +-
> 1 file changed, 21 insertions(+), 13 deletions(-)
>
> diff --git a/drivers/staging/erofs/super.c b/drivers/staging/erofs/super.c
> index 9e421536cbdf..f496e0c1d04d 100644
> --- a/drivers/staging/erofs/super.c
> +++ b/drivers/staging/erofs/super.c
> @@ -145,10 +145,14 @@ char *erofs_fault_name[FAULT_MAX] = {
> [FAULT_KMALLOC] = "kmalloc",
> };
>
> -static void erofs_build_fault_attr(struct erofs_sb_info *sbi,
> - unsigned int rate)
> +static int erofs_build_fault_attr(struct erofs_sb_info *sbi,
> + substring_t *args)
minor... two checkpatch suggestions out there, it is better to fix
them before merging...
CHECK: Alignment should match open parenthesis
#143: FILE: drivers/staging/erofs/super.c:149:
+static int erofs_build_fault_attr(struct erofs_sb_info *sbi,
+ substring_t *args)
CHECK: Alignment should match open parenthesis
#163: FILE: drivers/staging/erofs/super.c:170:
+static int erofs_build_fault_attr(struct erofs_sb_info *sbi,
+ substring_t *args)
and you could add
Reviewed-by: Gao Xiang
Thanks,
Gao Xiang
> {
> struct erofs_fault_info *ffi = &sbi->fault_info;
> + int rate = 0;
> +
> + if (args->from && match_int(args, &rate))
> + return -EINVAL;
>
> if (rate) {
> atomic_set(&ffi->inject_ops, 0);
> @@ -157,6 +161,16 @@ static void erofs_build_fault_attr(struct erofs_sb_info
> *sbi,
> } else {
> memset(ffi, 0, sizeof(struct erofs_fault_info));
> }
> +
> + set_opt(sbi, FAULT_INJECTION);
> + return 0;
> +}
> +#else
> +static int erofs_build_fault_attr(struct erofs_sb_info *sbi,
> + substring_t *args)
> +{
> + infoln("fault_injection options not supported");
> + return 0;
> }
> #endif
>
> @@ -193,7 +207,7 @@ static int parse_options(struct super_block *sb, char
> *options)
> {
> substring_t args[MAX_OPT_ARGS];
> char *p;
> - int arg = 0;
> + int err;
>
> if (!options)
> return 0;
> @@ -238,18 +252,12 @@ static int parse_options(struct super_block *sb, char
> *options)
> infoln("noacl options not supported");
> break;
> #endif
> -#ifdef CONFIG_EROFS_FAULT_INJECTION
> case Opt_fault_injection:
> - if (args->from && match_int(args, &arg))
> - return -EINVAL;
> - erofs_build_fault_attr(EROFS_SB(sb), arg);
> - set_opt(EROFS_SB(sb), FAULT_INJECTION);
> + err = erofs_build_fault_attr(EROFS_SB(sb), args);
> + if (err)
> + return err;
> break;
> -#else
> - case Opt_fault_injection:
> - infoln("fault_injection options not supported");
> - break;
> -#endif
> +
> default:
> errln("Unrecognized mount option \"%s\" "
> "or missing value", p);
>
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH v3 3/4] staging: erofs: code cleanup for erofs_show_options()
On 2018/9/18 23:10, Chengguang Xu wrote: > Add new helper erofs_get_fault_rate() to get fault rate instead of > directly getting it from sbi, so we can remove the macro check > surrounding it. > > Signed-off-by: Chengguang Xu > Reviewed-by: Chao Yu Reviewed-by: Gao Xiang Thanks, Gao Xiang ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH v3 4/4] staging: erofs: option validation in remount
Hi Chengguang,
On 2018/9/18 23:10, Chengguang Xu wrote:
> Add option validation in remount. After this patch, remount
> can change recognized options, and for unknown options remount
> will fail and report error.
>
> Signed-off-by: Chengguang Xu
> Reviewed-by: Chao Yu
> ---
> drivers/staging/erofs/super.c | 38 ---
> 1 file changed, 31 insertions(+), 7 deletions(-)
>
> diff --git a/drivers/staging/erofs/super.c b/drivers/staging/erofs/super.c
> index a091b93190e1..30b6b44dc6c4 100644
> --- a/drivers/staging/erofs/super.c
> +++ b/drivers/staging/erofs/super.c
> @@ -145,14 +145,10 @@ char *erofs_fault_name[FAULT_MAX] = {
> [FAULT_KMALLOC] = "kmalloc",
> };
>
> -static int erofs_build_fault_attr(struct erofs_sb_info *sbi,
> - substring_t *args)
> +static void __erofs_build_fault_attr(struct erofs_sb_info *sbi,
> + unsigned int rate)
> {
> struct erofs_fault_info *ffi = &sbi->fault_info;
> - int rate = 0;
> -
> - if (args->from && match_int(args, &rate))
> - return -EINVAL;
>
> if (rate) {
> atomic_set(&ffi->inject_ops, 0);
> @@ -163,14 +159,29 @@ static int erofs_build_fault_attr(struct erofs_sb_info
> *sbi,
> }
>
> set_opt(sbi, FAULT_INJECTION);
> - return 0;
> }
>
> static unsigned int erofs_get_fault_rate(struct erofs_sb_info *sbi)
> {
> return sbi->fault_info.inject_rate;
> }
> +
> +static int erofs_build_fault_attr(struct erofs_sb_info *sbi, substring_t
> *args)
> +{
> + int rate = 0;
> +
> + if (args->from && match_int(args, &rate))
> + return -EINVAL;
> +
> + __erofs_build_fault_attr(sbi, rate);
> + return 0;
> +}
> #else
> +static void __erofs_build_fault_attr(struct erofs_sb_info *sbi,
> + unsigned int rate)
> +{
> +}
> +
> static int erofs_build_fault_attr(struct erofs_sb_info *sbi,
> substring_t *args)
> {
> @@ -644,10 +655,23 @@ static int erofs_show_options(struct seq_file *seq,
> struct dentry *root)
>
> static int erofs_remount(struct super_block *sb, int *flags, char *data)
> {
> + struct erofs_sb_info *sbi = EROFS_SB(sb);
> + unsigned int org_mnt_opt = sbi->mount_opt;
> + unsigned int org_inject_rate = erofs_get_fault_rate(sbi);
> + int err;
> +
> BUG_ON(!sb_rdonly(sb));
> + err = parse_options(sb, data);
> + if (err)
> + goto out;
>
> *flags |= MS_RDONLY;
I cannot apply this patch since the above line has been changed in
5f0abea6ab6d("staging: erofs: rename superblock flags (MS_xyz -> SB_xyz)").
And two more checkpatch CHECKs as [PATCH v3 2/4]
CHECK: Alignment should match open parenthesis
#141: FILE: drivers/staging/erofs/super.c:149:
+static void __erofs_build_fault_attr(struct erofs_sb_info *sbi,
+ unsigned int rate)
CHECK: Alignment should match open parenthesis
#175: FILE: drivers/staging/erofs/super.c:181:
+static void __erofs_build_fault_attr(struct erofs_sb_info *sbi,
+ unsigned int rate)
And I think I need to test the basic erofs remount function tomorrow
before merging... it is too late and I have to go to sleep now...
Thanks,
Gao Xiang
> return 0;
> +out:
> + __erofs_build_fault_attr(sbi, org_inject_rate);
> + sbi->mount_opt = org_mnt_opt;
> +
> + return err;
> }
>
> const struct super_operations erofs_sops = {
>
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH 00/12] staging: vboxvideo: Preparation work for moving to atomic modesetting
Hi Greg, Here is a series of various cleanups and other prep. work for moving the vboxvideo driver over to atomic modesetting so that it can be moved out of staging. Regards, Hans ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH 02/12] staging: vboxvideo: Move setup of modesetting from driver_load to mode_init
Move the setup of drm modesetting config from vbox_driver_load() to
vbox_mode_init().
Signed-off-by: Hans de Goede
---
drivers/staging/vboxvideo/vbox_main.c | 45 ---
drivers/staging/vboxvideo/vbox_mode.c | 62 ---
2 files changed, 57 insertions(+), 50 deletions(-)
diff --git a/drivers/staging/vboxvideo/vbox_main.c
b/drivers/staging/vboxvideo/vbox_main.c
index 783a68c0de3b..a1cd29fe98fd 100644
--- a/drivers/staging/vboxvideo/vbox_main.c
+++ b/drivers/staging/vboxvideo/vbox_main.c
@@ -173,40 +173,6 @@ int vbox_framebuffer_init(struct drm_device *dev,
return 0;
}
-static struct drm_framebuffer *vbox_user_framebuffer_create(
- struct drm_device *dev,
- struct drm_file *filp,
- const struct drm_mode_fb_cmd2 *mode_cmd)
-{
- struct drm_gem_object *obj;
- struct vbox_framebuffer *vbox_fb;
- int ret = -ENOMEM;
-
- obj = drm_gem_object_lookup(filp, mode_cmd->handles[0]);
- if (!obj)
- return ERR_PTR(-ENOENT);
-
- vbox_fb = kzalloc(sizeof(*vbox_fb), GFP_KERNEL);
- if (!vbox_fb)
- goto err_unref_obj;
-
- ret = vbox_framebuffer_init(dev, vbox_fb, mode_cmd, obj);
- if (ret)
- goto err_free_vbox_fb;
-
- return &vbox_fb->base;
-
-err_free_vbox_fb:
- kfree(vbox_fb);
-err_unref_obj:
- drm_gem_object_put_unlocked(obj);
- return ERR_PTR(ret);
-}
-
-static const struct drm_mode_config_funcs vbox_mode_funcs = {
- .fb_create = vbox_user_framebuffer_create,
-};
-
static int vbox_accel_init(struct vbox_private *vbox)
{
unsigned int i;
@@ -375,15 +341,6 @@ int vbox_driver_load(struct drm_device *dev)
if (ret)
goto err_hw_fini;
- drm_mode_config_init(dev);
-
- dev->mode_config.funcs = (void *)&vbox_mode_funcs;
- dev->mode_config.min_width = 64;
- dev->mode_config.min_height = 64;
- dev->mode_config.preferred_depth = 24;
- dev->mode_config.max_width = VBE_DISPI_MAX_XRES;
- dev->mode_config.max_height = VBE_DISPI_MAX_YRES;
-
ret = vbox_mode_init(dev);
if (ret)
goto err_drm_mode_cleanup;
@@ -403,7 +360,6 @@ int vbox_driver_load(struct drm_device *dev)
err_mode_fini:
vbox_mode_fini(dev);
err_drm_mode_cleanup:
- drm_mode_config_cleanup(dev);
vbox_mm_fini(vbox);
err_hw_fini:
vbox_hw_fini(vbox);
@@ -417,7 +373,6 @@ void vbox_driver_unload(struct drm_device *dev)
vbox_fbdev_fini(dev);
vbox_irq_fini(vbox);
vbox_mode_fini(dev);
- drm_mode_config_cleanup(dev);
vbox_mm_fini(vbox);
vbox_hw_fini(vbox);
}
diff --git a/drivers/staging/vboxvideo/vbox_mode.c
b/drivers/staging/vboxvideo/vbox_mode.c
index 70701a6054c2..2587e6aecca2 100644
--- a/drivers/staging/vboxvideo/vbox_mode.c
+++ b/drivers/staging/vboxvideo/vbox_mode.c
@@ -696,6 +696,40 @@ static int vbox_connector_init(struct drm_device *dev,
return 0;
}
+static struct drm_framebuffer *vbox_user_framebuffer_create(
+ struct drm_device *dev,
+ struct drm_file *filp,
+ const struct drm_mode_fb_cmd2 *mode_cmd)
+{
+ struct drm_gem_object *obj;
+ struct vbox_framebuffer *vbox_fb;
+ int ret = -ENOMEM;
+
+ obj = drm_gem_object_lookup(filp, mode_cmd->handles[0]);
+ if (!obj)
+ return ERR_PTR(-ENOENT);
+
+ vbox_fb = kzalloc(sizeof(*vbox_fb), GFP_KERNEL);
+ if (!vbox_fb)
+ goto err_unref_obj;
+
+ ret = vbox_framebuffer_init(dev, vbox_fb, mode_cmd, obj);
+ if (ret)
+ goto err_free_vbox_fb;
+
+ return &vbox_fb->base;
+
+err_free_vbox_fb:
+ kfree(vbox_fb);
+err_unref_obj:
+ drm_gem_object_put_unlocked(obj);
+ return ERR_PTR(ret);
+}
+
+static const struct drm_mode_config_funcs vbox_mode_funcs = {
+ .fb_create = vbox_user_framebuffer_create,
+};
+
int vbox_mode_init(struct drm_device *dev)
{
struct vbox_private *vbox = dev->dev_private;
@@ -704,24 +738,42 @@ int vbox_mode_init(struct drm_device *dev)
unsigned int i;
int ret;
+ drm_mode_config_init(dev);
+
+ dev->mode_config.funcs = (void *)&vbox_mode_funcs;
+ dev->mode_config.min_width = 64;
+ dev->mode_config.min_height = 64;
+ dev->mode_config.preferred_depth = 24;
+ dev->mode_config.max_width = VBE_DISPI_MAX_XRES;
+ dev->mode_config.max_height = VBE_DISPI_MAX_YRES;
+
/* vbox_cursor_init(dev); */
for (i = 0; i < vbox->num_crtcs; ++i) {
vbox_crtc = vbox_crtc_init(dev, i);
- if (!vbox_crtc)
- return -ENOMEM;
+ if (!vbox_crtc) {
+ ret = -ENOMEM;
+ goto err_drm_mode_cleanup;
+ }
encoder = vbox_encoder_init(dev, i);
- if (!encoder)
-
[PATCH 03/12] staging: vboxvideo: Fold driver_load/unload into probe/remove functions
Fold the driver_load / unload functions into the probe / remove functions
now that we are no longer using the deprecated drm_get_pci_dev() mechanism.
Signed-off-by: Hans de Goede
---
drivers/staging/vboxvideo/vbox_drv.c | 71 +--
drivers/staging/vboxvideo/vbox_drv.h | 6 ++-
drivers/staging/vboxvideo/vbox_main.c | 67 ++---
3 files changed, 63 insertions(+), 81 deletions(-)
diff --git a/drivers/staging/vboxvideo/vbox_drv.c
b/drivers/staging/vboxvideo/vbox_drv.c
index 69cc508af1bc..410a1f35b746 100644
--- a/drivers/staging/vboxvideo/vbox_drv.c
+++ b/drivers/staging/vboxvideo/vbox_drv.c
@@ -51,48 +51,89 @@ MODULE_DEVICE_TABLE(pci, pciidlist);
static int vbox_pci_probe(struct pci_dev *pdev, const struct pci_device_id
*ent)
{
+ struct vbox_private *vbox = NULL;
struct drm_device *dev = NULL;
int ret = 0;
+ if (!vbox_check_supported(VBE_DISPI_ID_HGSMI))
+ return -ENODEV;
+
dev = drm_dev_alloc(&driver, &pdev->dev);
- if (IS_ERR(dev)) {
- ret = PTR_ERR(dev);
- goto err_drv_alloc;
- }
+ if (IS_ERR(dev))
+ return PTR_ERR(dev);
ret = pci_enable_device(pdev);
if (ret)
- goto err_pci_enable;
+ goto err_dev_put;
dev->pdev = pdev;
pci_set_drvdata(pdev, dev);
- ret = vbox_driver_load(dev);
+ vbox = devm_kzalloc(&pdev->dev, sizeof(*vbox), GFP_KERNEL);
+ if (!vbox) {
+ ret = -ENOMEM;
+ goto err_pci_disable;
+ }
+
+ dev->dev_private = vbox;
+ vbox->dev = dev;
+
+ mutex_init(&vbox->hw_mutex);
+
+ ret = vbox_hw_init(vbox);
+ if (ret)
+ goto err_pci_disable;
+
+ ret = vbox_mm_init(vbox);
if (ret)
- goto err_vbox_driver_load;
+ goto err_hw_fini;
+
+ ret = vbox_mode_init(dev);
+ if (ret)
+ goto err_mm_fini;
+
+ ret = vbox_irq_init(vbox);
+ if (ret)
+ goto err_mode_fini;
+
+ ret = vbox_fbdev_init(dev);
+ if (ret)
+ goto err_irq_fini;
ret = drm_dev_register(dev, 0);
if (ret)
- goto err_drv_dev_register;
+ goto err_fbdev_fini;
- return ret;
+ return 0;
- err_drv_dev_register:
- vbox_driver_unload(dev);
- err_vbox_driver_load:
+err_fbdev_fini:
+ vbox_fbdev_fini(dev);
+err_irq_fini:
+ vbox_irq_fini(vbox);
+err_mode_fini:
+ vbox_mode_fini(dev);
+err_mm_fini:
+ vbox_mm_fini(vbox);
+err_hw_fini:
+ vbox_hw_fini(vbox);
+err_pci_disable:
pci_disable_device(pdev);
- err_pci_enable:
+err_dev_put:
drm_dev_put(dev);
- err_drv_alloc:
return ret;
}
static void vbox_pci_remove(struct pci_dev *pdev)
{
struct drm_device *dev = pci_get_drvdata(pdev);
+ struct vbox_private *vbox = dev->dev_private;
drm_dev_unregister(dev);
- vbox_driver_unload(dev);
+ vbox_fbdev_fini(dev);
+ vbox_irq_fini(vbox);
+ vbox_mode_fini(dev);
+ vbox_mm_fini(vbox);
+ vbox_hw_fini(vbox);
drm_dev_put(dev);
}
diff --git a/drivers/staging/vboxvideo/vbox_drv.h
b/drivers/staging/vboxvideo/vbox_drv.h
index 594f84272957..a8e0dd8b57bf 100644
--- a/drivers/staging/vboxvideo/vbox_drv.h
+++ b/drivers/staging/vboxvideo/vbox_drv.h
@@ -126,8 +126,6 @@ struct vbox_private {
#undef CURSOR_PIXEL_COUNT
#undef CURSOR_DATA_SIZE
-int vbox_driver_load(struct drm_device *dev);
-void vbox_driver_unload(struct drm_device *dev);
void vbox_driver_lastclose(struct drm_device *dev);
struct vbox_gem_object;
@@ -177,6 +175,10 @@ struct vbox_fbdev {
#define to_vbox_encoder(x) container_of(x, struct vbox_encoder, base)
#define to_vbox_framebuffer(x) container_of(x, struct vbox_framebuffer, base)
+bool vbox_check_supported(u16 id);
+int vbox_hw_init(struct vbox_private *vbox);
+void vbox_hw_fini(struct vbox_private *vbox);
+
int vbox_mode_init(struct drm_device *dev);
void vbox_mode_fini(struct drm_device *dev);
diff --git a/drivers/staging/vboxvideo/vbox_main.c
b/drivers/staging/vboxvideo/vbox_main.c
index a1cd29fe98fd..815292f1d7e6 100644
--- a/drivers/staging/vboxvideo/vbox_main.c
+++ b/drivers/staging/vboxvideo/vbox_main.c
@@ -228,7 +228,7 @@ static bool have_hgsmi_mode_hints(struct vbox_private *vbox)
return have_hints == VINF_SUCCESS && have_cursor == VINF_SUCCESS;
}
-static bool vbox_check_supported(u16 id)
+bool vbox_check_supported(u16 id)
{
u16 dispi_id;
@@ -242,7 +242,7 @@ static bool vbox_check_supported(u16 id)
* Set up our heaps and data exchange buffers in VRAM before handing the rest
* to the memory manager.
*/
-static int vbox_hw_init(struct vbox_private *vbox)
+int vbox_hw_init(struct vbox_private *vbox)
{
int ret = -ENOMEM;
@@ -309,74 +309,13 @@ static int vbox_hw_init(struct vbox_private *vbox)
[PATCH 05/12] staging: vboxvideo: Fold vbox_drm_resume() into vbox_pm_resume()
vbox_pm_resume() is the only caller of vbox_drm_resume(), so squash the
2 functions into 1.
Signed-off-by: Hans de Goede
---
drivers/staging/vboxvideo/vbox_drv.c | 28 +++-
1 file changed, 11 insertions(+), 17 deletions(-)
diff --git a/drivers/staging/vboxvideo/vbox_drv.c
b/drivers/staging/vboxvideo/vbox_drv.c
index c4290d4b4a53..c6a53b0ad66c 100644
--- a/drivers/staging/vboxvideo/vbox_drv.c
+++ b/drivers/staging/vboxvideo/vbox_drv.c
@@ -152,22 +152,6 @@ static int vbox_drm_thaw(struct vbox_private *vbox)
return 0;
}
-static int vbox_drm_resume(struct vbox_private *vbox)
-{
- int ret;
-
- if (pci_enable_device(vbox->ddev.pdev))
- return -EIO;
-
- ret = vbox_drm_thaw(vbox);
- if (ret)
- return ret;
-
- drm_kms_helper_poll_enable(&vbox->ddev);
-
- return 0;
-}
-
static int vbox_pm_suspend(struct device *dev)
{
struct vbox_private *vbox = dev_get_drvdata(dev);
@@ -186,8 +170,18 @@ static int vbox_pm_suspend(struct device *dev)
static int vbox_pm_resume(struct device *dev)
{
struct vbox_private *vbox = dev_get_drvdata(dev);
+ int ret;
- return vbox_drm_resume(vbox);
+ if (pci_enable_device(vbox->ddev.pdev))
+ return -EIO;
+
+ ret = vbox_drm_thaw(vbox);
+ if (ret)
+ return ret;
+
+ drm_kms_helper_poll_enable(&vbox->ddev);
+
+ return 0;
}
static int vbox_pm_freeze(struct device *dev)
--
2.19.0.rc1
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH 07/12] staging: vboxvideo: Expose creation of universal primary plane
Let's expose the primary plane initialization inside the vboxvideo driver
in preparation for universal planes and atomic.
Signed-off-by: Hans de Goede
---
drivers/staging/vboxvideo/vbox_mode.c | 78 +--
1 file changed, 74 insertions(+), 4 deletions(-)
diff --git a/drivers/staging/vboxvideo/vbox_mode.c
b/drivers/staging/vboxvideo/vbox_mode.c
index 3beae9d65a09..f35045ce154b 100644
--- a/drivers/staging/vboxvideo/vbox_mode.c
+++ b/drivers/staging/vboxvideo/vbox_mode.c
@@ -382,21 +382,91 @@ static const struct drm_crtc_funcs vbox_crtc_funcs = {
.destroy = vbox_crtc_destroy,
};
+static const uint32_t vbox_primary_plane_formats[] = {
+ DRM_FORMAT_XRGB,
+ DRM_FORMAT_ARGB,
+};
+
+static const struct drm_plane_funcs vbox_primary_plane_funcs = {
+ .update_plane = drm_primary_helper_update,
+ .disable_plane = drm_primary_helper_disable,
+ .destroy= drm_primary_helper_destroy,
+};
+
+static struct drm_plane *vbox_create_plane(struct vbox_private *vbox,
+ unsigned int possible_crtcs,
+ enum drm_plane_type type)
+{
+ const struct drm_plane_helper_funcs *helper_funcs = NULL;
+ const struct drm_plane_funcs *funcs;
+ struct drm_plane *plane;
+ const uint32_t *formats;
+ int num_formats;
+ int err;
+
+ if (type == DRM_PLANE_TYPE_PRIMARY) {
+ funcs = &vbox_primary_plane_funcs;
+ formats = vbox_primary_plane_formats;
+ num_formats = ARRAY_SIZE(vbox_primary_plane_formats);
+ } else {
+ return ERR_PTR(-EINVAL);
+ }
+
+ plane = kzalloc(sizeof(*plane), GFP_KERNEL);
+ if (!plane)
+ return ERR_PTR(-ENOMEM);
+
+ err = drm_universal_plane_init(&vbox->ddev, plane, possible_crtcs,
+ funcs, formats, num_formats,
+ NULL, type, NULL);
+ if (err)
+ goto free_plane;
+
+ drm_plane_helper_add(plane, helper_funcs);
+
+ return plane;
+
+free_plane:
+ kfree(plane);
+ return ERR_PTR(-EINVAL);
+}
+
static struct vbox_crtc *vbox_crtc_init(struct drm_device *dev, unsigned int i)
{
+ struct vbox_private *vbox =
+ container_of(dev, struct vbox_private, ddev);
struct vbox_crtc *vbox_crtc;
+ struct drm_plane *primary;
+ int ret;
vbox_crtc = kzalloc(sizeof(*vbox_crtc), GFP_KERNEL);
if (!vbox_crtc)
- return NULL;
+ return ERR_PTR(-ENOMEM);
+
+ primary = vbox_create_plane(vbox, 1 << i, DRM_PLANE_TYPE_PRIMARY);
+ if (IS_ERR(primary)) {
+ ret = PTR_ERR(primary);
+ goto free_mem;
+ }
vbox_crtc->crtc_id = i;
- drm_crtc_init(dev, &vbox_crtc->base, &vbox_crtc_funcs);
+ ret = drm_crtc_init_with_planes(dev, &vbox_crtc->base, primary, NULL,
+ &vbox_crtc_funcs, NULL);
+ if (ret)
+ goto clean_primary;
+
drm_mode_crtc_set_gamma_size(&vbox_crtc->base, 256);
drm_crtc_helper_add(&vbox_crtc->base, &vbox_crtc_helper_funcs);
return vbox_crtc;
+
+clean_primary:
+ drm_plane_cleanup(primary);
+ kfree(primary);
+free_mem:
+ kfree(vbox_crtc);
+ return ERR_PTR(ret);
}
static void vbox_encoder_destroy(struct drm_encoder *encoder)
@@ -750,8 +820,8 @@ int vbox_mode_init(struct vbox_private *vbox)
/* vbox_cursor_init(dev); */
for (i = 0; i < vbox->num_crtcs; ++i) {
vbox_crtc = vbox_crtc_init(dev, i);
- if (!vbox_crtc) {
- ret = -ENOMEM;
+ if (IS_ERR(vbox_crtc)) {
+ ret = PTR_ERR(vbox_crtc);
goto err_drm_mode_cleanup;
}
encoder = vbox_encoder_init(dev, i);
--
2.19.0.rc1
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH 06/12] staging: vboxvideo: Add fl_flag argument to vbox_fb_pin() helper
Allow specifying where to pin the framebuffer bo, so that this helper can
be used from the cursor code too.
Signed-off-by: Hans de Goede
---
drivers/staging/vboxvideo/vbox_mode.c | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/drivers/staging/vboxvideo/vbox_mode.c
b/drivers/staging/vboxvideo/vbox_mode.c
index 13696ba19c4f..3beae9d65a09 100644
--- a/drivers/staging/vboxvideo/vbox_mode.c
+++ b/drivers/staging/vboxvideo/vbox_mode.c
@@ -221,7 +221,7 @@ static bool vbox_set_up_input_mapping(struct vbox_private
*vbox)
return old_single_framebuffer != vbox->single_framebuffer;
}
-static int vbox_fb_pin(struct drm_framebuffer *fb, u64 *addr)
+static int vbox_fb_pin(struct drm_framebuffer *fb, u32 pl_flag, u64 *addr)
{
struct vbox_bo *bo = gem_to_vbox_bo(to_vbox_framebuffer(fb)->obj);
int ret;
@@ -230,7 +230,7 @@ static int vbox_fb_pin(struct drm_framebuffer *fb, u64
*addr)
if (ret)
return ret;
- ret = vbox_bo_pin(bo, TTM_PL_FLAG_VRAM, addr);
+ ret = vbox_bo_pin(bo, pl_flag, addr);
vbox_bo_unreserve(bo);
return ret;
}
@@ -267,7 +267,7 @@ static int vbox_crtc_set_base_and_mode(struct drm_crtc
*crtc,
int ret;
/* Prepare: pin the new framebuffer bo */
- ret = vbox_fb_pin(new_fb, &gpu_addr);
+ ret = vbox_fb_pin(new_fb, TTM_PL_FLAG_VRAM, &gpu_addr);
if (ret) {
DRM_WARN("Error %d pinning new fb, out of video mem?\n", ret);
return ret;
--
2.19.0.rc1
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH 01/12] staging: vboxvideo: Let DRM core handle connector registering
Registering the connector explicitly right after creation is not necessary for modesetting drivers, because drm_dev_register already takes care of this on the core side, by calling drm_modeset_register_all. Signed-off-by: Hans de Goede --- drivers/staging/vboxvideo/vbox_mode.c | 1 - 1 file changed, 1 deletion(-) diff --git a/drivers/staging/vboxvideo/vbox_mode.c b/drivers/staging/vboxvideo/vbox_mode.c index e7d70ced5bfd..70701a6054c2 100644 --- a/drivers/staging/vboxvideo/vbox_mode.c +++ b/drivers/staging/vboxvideo/vbox_mode.c @@ -690,7 +690,6 @@ static int vbox_connector_init(struct drm_device *dev, dev->mode_config.suggested_x_property, 0); drm_object_attach_property(&connector->base, dev->mode_config.suggested_y_property, 0); - drm_connector_register(connector); drm_connector_attach_encoder(connector, encoder); -- 2.19.0.rc1 ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH 09/12] staging: vboxvideo: Move pin / unpin of fb out of vbox_crtc_set_base_and_mode
Move pin / unpin of fb out of vbox_crtc_set_base_and_mode() so that it can
be used to implement the atomic_update drm_plane_helper_func for the primary
plane.
Signed-off-by: Hans de Goede
---
drivers/staging/vboxvideo/vbox_drv.h | 5 +++
drivers/staging/vboxvideo/vbox_mode.c | 52 ++-
drivers/staging/vboxvideo/vbox_ttm.c | 5 ---
3 files changed, 32 insertions(+), 30 deletions(-)
diff --git a/drivers/staging/vboxvideo/vbox_drv.h
b/drivers/staging/vboxvideo/vbox_drv.h
index 5034c6bd5445..6a4d3b382e79 100644
--- a/drivers/staging/vboxvideo/vbox_drv.h
+++ b/drivers/staging/vboxvideo/vbox_drv.h
@@ -220,6 +220,11 @@ static inline struct vbox_bo *vbox_bo(struct
ttm_buffer_object *bo)
#define to_vbox_obj(x) container_of(x, struct vbox_gem_object, base)
+static inline u64 vbox_bo_gpu_offset(struct vbox_bo *bo)
+{
+ return bo->bo.offset;
+}
+
int vbox_dumb_create(struct drm_file *file,
struct drm_device *dev,
struct drm_mode_create_dumb *args);
diff --git a/drivers/staging/vboxvideo/vbox_mode.c
b/drivers/staging/vboxvideo/vbox_mode.c
index 6d7a89524fbf..1a2416a59fe0 100644
--- a/drivers/staging/vboxvideo/vbox_mode.c
+++ b/drivers/staging/vboxvideo/vbox_mode.c
@@ -255,28 +255,18 @@ static void vbox_fb_unpin(struct drm_framebuffer *fb)
vbox_bo_unreserve(bo);
}
-static int vbox_crtc_set_base_and_mode(struct drm_crtc *crtc,
- struct drm_framebuffer *old_fb,
- struct drm_framebuffer *new_fb,
- struct drm_display_mode *mode,
- int x, int y)
+static void vbox_crtc_set_base_and_mode(struct drm_crtc *crtc,
+ struct drm_framebuffer *fb,
+ struct drm_display_mode *mode,
+ int x, int y)
{
+ struct vbox_bo *bo = gem_to_vbox_bo(to_vbox_framebuffer(fb)->obj);
struct vbox_private *vbox = crtc->dev->dev_private;
struct vbox_crtc *vbox_crtc = to_vbox_crtc(crtc);
- u64 gpu_addr;
- int ret;
-
- /* Prepare: pin the new framebuffer bo */
- ret = vbox_fb_pin(new_fb, TTM_PL_FLAG_VRAM, &gpu_addr);
- if (ret) {
- DRM_WARN("Error %d pinning new fb, out of video mem?\n", ret);
- return ret;
- }
- /* Commit: Update hardware to use the new fb */
mutex_lock(&vbox->hw_mutex);
- vbox_crtc->fb_offset = gpu_addr;
+ vbox_crtc->fb_offset = vbox_bo_gpu_offset(bo);
/* vbox_do_modeset() checks vbox->single_framebuffer so update it now */
if (mode && vbox_set_up_input_mapping(vbox)) {
@@ -299,11 +289,6 @@ static int vbox_crtc_set_base_and_mode(struct drm_crtc
*crtc,
vbox->input_mapping_height);
mutex_unlock(&vbox->hw_mutex);
-
- /* Cleanup: unpin the old fb */
- vbox_fb_unpin(old_fb);
-
- return 0;
}
static int vbox_crtc_mode_set(struct drm_crtc *crtc,
@@ -311,8 +296,19 @@ static int vbox_crtc_mode_set(struct drm_crtc *crtc,
struct drm_display_mode *adjusted_mode,
int x, int y, struct drm_framebuffer *old_fb)
{
- return vbox_crtc_set_base_and_mode(crtc, old_fb, CRTC_FB(crtc),
- mode, x, y);
+ int ret;
+
+ ret = vbox_fb_pin(CRTC_FB(crtc), TTM_PL_FLAG_VRAM, NULL);
+ if (ret) {
+ DRM_WARN("Error %d pinning new fb, out of video mem?\n", ret);
+ return ret;
+ }
+
+ vbox_crtc_set_base_and_mode(crtc, CRTC_FB(crtc), mode, x, y);
+
+ vbox_fb_unpin(old_fb);
+
+ return 0;
}
static int vbox_crtc_page_flip(struct drm_crtc *crtc,
@@ -324,9 +320,15 @@ static int vbox_crtc_page_flip(struct drm_crtc *crtc,
unsigned long flags;
int rc;
- rc = vbox_crtc_set_base_and_mode(crtc, CRTC_FB(crtc), fb, NULL, 0, 0);
- if (rc)
+ rc = vbox_fb_pin(fb, TTM_PL_FLAG_VRAM, NULL);
+ if (rc) {
+ DRM_WARN("Error %d pinning new fb, out of video mem?\n", rc);
return rc;
+ }
+
+ vbox_crtc_set_base_and_mode(crtc, fb, NULL, 0, 0);
+
+ vbox_fb_unpin(CRTC_FB(crtc));
spin_lock_irqsave(&crtc->dev->event_lock, flags);
diff --git a/drivers/staging/vboxvideo/vbox_ttm.c
b/drivers/staging/vboxvideo/vbox_ttm.c
index 7b8eac30faca..0e14556dcd6b 100644
--- a/drivers/staging/vboxvideo/vbox_ttm.c
+++ b/drivers/staging/vboxvideo/vbox_ttm.c
@@ -343,11 +343,6 @@ int vbox_bo_create(struct vbox_private *vbox, int size,
int align,
return ret;
}
-static inline u64 vbox_bo_gpu_offset(struct vbox_bo *bo)
-{
- return bo->bo.offset;
-}
-
int vbox_bo_pin(struct vbox_bo *bo, u32 pl_flag, u64 *gpu_addr)
{
struct ttm_operation_ctx ctx = { false, false };
--
[PATCH 08/12] staging: vboxvideo: Init fb_info.fix.smem once from fbdev_create
The fbdev compat fb gets pinned into VRAM at creation and then gets pinned
a second time when set as scanout buffer and unpinned when replaced, this
means its pin count never becomes less then 1, so it is always at the same
address and there is no need for the vbox_fbdev_set_base() call.
Signed-off-by: Hans de Goede
---
drivers/staging/vboxvideo/vbox_drv.h | 1 -
drivers/staging/vboxvideo/vbox_fb.c | 14 +-
drivers/staging/vboxvideo/vbox_mode.c | 3 ---
3 files changed, 5 insertions(+), 13 deletions(-)
diff --git a/drivers/staging/vboxvideo/vbox_drv.h
b/drivers/staging/vboxvideo/vbox_drv.h
index 28ffdffe877a..5034c6bd5445 100644
--- a/drivers/staging/vboxvideo/vbox_drv.h
+++ b/drivers/staging/vboxvideo/vbox_drv.h
@@ -201,7 +201,6 @@ int vbox_framebuffer_init(struct vbox_private *vbox,
int vbox_fbdev_init(struct vbox_private *vbox);
void vbox_fbdev_fini(struct vbox_private *vbox);
-void vbox_fbdev_set_base(struct vbox_private *vbox, unsigned long gpu_addr);
struct vbox_bo {
struct ttm_buffer_object bo;
diff --git a/drivers/staging/vboxvideo/vbox_fb.c
b/drivers/staging/vboxvideo/vbox_fb.c
index 11b6364ed14a..0e5550fa7c57 100644
--- a/drivers/staging/vboxvideo/vbox_fb.c
+++ b/drivers/staging/vboxvideo/vbox_fb.c
@@ -80,6 +80,7 @@ static int vboxfb_create(struct drm_fb_helper *helper,
struct drm_gem_object *gobj;
struct vbox_bo *bo;
int size, ret;
+ u64 gpu_addr;
u32 pitch;
mode_cmd.width = sizes->surface_width;
@@ -107,7 +108,7 @@ static int vboxfb_create(struct drm_fb_helper *helper,
if (ret)
return ret;
- ret = vbox_bo_pin(bo, TTM_PL_FLAG_VRAM, NULL);
+ ret = vbox_bo_pin(bo, TTM_PL_FLAG_VRAM, &gpu_addr);
if (ret) {
vbox_bo_unreserve(bo);
return ret;
@@ -152,6 +153,9 @@ static int vboxfb_create(struct drm_fb_helper *helper,
drm_fb_helper_fill_var(info, &fbdev->helper, sizes->fb_width,
sizes->fb_height);
+ info->fix.smem_start = info->apertures->ranges[0].base + gpu_addr;
+ info->fix.smem_len = vbox->available_vram_size - gpu_addr;
+
info->screen_base = (char __iomem *)bo->kmap.virtual;
info->screen_size = size;
@@ -241,11 +245,3 @@ int vbox_fbdev_init(struct vbox_private *vbox)
drm_fb_helper_fini(&fbdev->helper);
return ret;
}
-
-void vbox_fbdev_set_base(struct vbox_private *vbox, unsigned long gpu_addr)
-{
- struct fb_info *fbdev = vbox->fbdev->helper.fbdev;
-
- fbdev->fix.smem_start = fbdev->apertures->ranges[0].base + gpu_addr;
- fbdev->fix.smem_len = vbox->available_vram_size - gpu_addr;
-}
diff --git a/drivers/staging/vboxvideo/vbox_mode.c
b/drivers/staging/vboxvideo/vbox_mode.c
index f35045ce154b..6d7a89524fbf 100644
--- a/drivers/staging/vboxvideo/vbox_mode.c
+++ b/drivers/staging/vboxvideo/vbox_mode.c
@@ -276,9 +276,6 @@ static int vbox_crtc_set_base_and_mode(struct drm_crtc
*crtc,
/* Commit: Update hardware to use the new fb */
mutex_lock(&vbox->hw_mutex);
- if (&vbox->fbdev->afb == to_vbox_framebuffer(new_fb))
- vbox_fbdev_set_base(vbox, gpu_addr);
-
vbox_crtc->fb_offset = gpu_addr;
/* vbox_do_modeset() checks vbox->single_framebuffer so update it now */
--
2.19.0.rc1
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH 10/12] staging: vboxvideo: Fix NULL ptr deref in vbox_set_up_input_mapping()
When vbox_set_up_input_mapping() gets called the first crtc might be
disable and not have a fb at all, triggering a NUL ptr deref at:
vbox->input_mapping_width = CRTC_FB(crtci)->width;
Instead of using the fb from the crtc with id 0, just use the fb from
the first crtc with a fb. This is in the single_framebuffer = true path,
so all crtc-s point to the same fb anyways.
Signed-off-by: Hans de Goede
---
drivers/staging/vboxvideo/vbox_mode.c | 8
1 file changed, 4 insertions(+), 4 deletions(-)
diff --git a/drivers/staging/vboxvideo/vbox_mode.c
b/drivers/staging/vboxvideo/vbox_mode.c
index 1a2416a59fe0..910ea19931c9 100644
--- a/drivers/staging/vboxvideo/vbox_mode.c
+++ b/drivers/staging/vboxvideo/vbox_mode.c
@@ -189,17 +189,17 @@ static bool vbox_set_up_input_mapping(struct vbox_private
*vbox)
}
}
if (single_framebuffer) {
+ vbox->single_framebuffer = true;
list_for_each_entry(crtci, &vbox->ddev.mode_config.crtc_list,
head) {
- if (to_vbox_crtc(crtci)->crtc_id != 0)
+ if (!CRTC_FB(crtci))
continue;
- vbox->single_framebuffer = true;
vbox->input_mapping_width = CRTC_FB(crtci)->width;
vbox->input_mapping_height = CRTC_FB(crtci)->height;
- return old_single_framebuffer !=
- vbox->single_framebuffer;
+ break;
}
+ return old_single_framebuffer != vbox->single_framebuffer;
}
/* Otherwise calculate the total span of all screens. */
list_for_each_entry(connectori, &vbox->ddev.mode_config.connector_list,
--
2.19.0.rc1
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH 04/12] staging: vboxvideo: Embed drm_device into driver structure
This is the recommended way to create the drm_device structure,
according to DRM documentation.
Signed-off-by: Hans de Goede
---
drivers/staging/vboxvideo/vbox_drv.c | 110 +++---
drivers/staging/vboxvideo/vbox_drv.h | 17 ++--
drivers/staging/vboxvideo/vbox_fb.c | 19 ++---
drivers/staging/vboxvideo/vbox_irq.c | 8 +-
drivers/staging/vboxvideo/vbox_main.c | 30 +++
drivers/staging/vboxvideo/vbox_mode.c | 36 -
drivers/staging/vboxvideo/vbox_ttm.c | 13 ++-
7 files changed, 111 insertions(+), 122 deletions(-)
diff --git a/drivers/staging/vboxvideo/vbox_drv.c
b/drivers/staging/vboxvideo/vbox_drv.c
index 410a1f35b746..c4290d4b4a53 100644
--- a/drivers/staging/vboxvideo/vbox_drv.c
+++ b/drivers/staging/vboxvideo/vbox_drv.c
@@ -51,35 +51,31 @@ MODULE_DEVICE_TABLE(pci, pciidlist);
static int vbox_pci_probe(struct pci_dev *pdev, const struct pci_device_id
*ent)
{
- struct vbox_private *vbox = NULL;
- struct drm_device *dev = NULL;
+ struct vbox_private *vbox;
int ret = 0;
if (!vbox_check_supported(VBE_DISPI_ID_HGSMI))
return -ENODEV;
- dev = drm_dev_alloc(&driver, &pdev->dev);
- if (IS_ERR(dev))
- return PTR_ERR(dev);
+ vbox = kzalloc(sizeof(*vbox), GFP_KERNEL);
+ if (!vbox)
+ return -ENOMEM;
- ret = pci_enable_device(pdev);
- if (ret)
- goto err_dev_put;
-
- dev->pdev = pdev;
- pci_set_drvdata(pdev, dev);
-
- vbox = devm_kzalloc(&pdev->dev, sizeof(*vbox), GFP_KERNEL);
- if (!vbox) {
- ret = -ENOMEM;
- goto err_pci_disable;
+ ret = drm_dev_init(&vbox->ddev, &driver, &pdev->dev);
+ if (ret) {
+ kfree(vbox);
+ return ret;
}
- dev->dev_private = vbox;
- vbox->dev = dev;
-
+ vbox->ddev.pdev = pdev;
+ vbox->ddev.dev_private = vbox;
+ pci_set_drvdata(pdev, vbox);
mutex_init(&vbox->hw_mutex);
+ ret = pci_enable_device(pdev);
+ if (ret)
+ goto err_dev_put;
+
ret = vbox_hw_init(vbox);
if (ret)
goto err_pci_disable;
@@ -88,7 +84,7 @@ static int vbox_pci_probe(struct pci_dev *pdev, const struct
pci_device_id *ent)
if (ret)
goto err_hw_fini;
- ret = vbox_mode_init(dev);
+ ret = vbox_mode_init(vbox);
if (ret)
goto err_mm_fini;
@@ -96,22 +92,22 @@ static int vbox_pci_probe(struct pci_dev *pdev, const
struct pci_device_id *ent)
if (ret)
goto err_mode_fini;
- ret = vbox_fbdev_init(dev);
+ ret = vbox_fbdev_init(vbox);
if (ret)
goto err_irq_fini;
- ret = drm_dev_register(dev, 0);
+ ret = drm_dev_register(&vbox->ddev, 0);
if (ret)
goto err_fbdev_fini;
return 0;
err_fbdev_fini:
- vbox_fbdev_fini(dev);
+ vbox_fbdev_fini(vbox);
err_irq_fini:
vbox_irq_fini(vbox);
err_mode_fini:
- vbox_mode_fini(dev);
+ vbox_mode_fini(vbox);
err_mm_fini:
vbox_mm_fini(vbox);
err_hw_fini:
@@ -119,110 +115,100 @@ static int vbox_pci_probe(struct pci_dev *pdev, const
struct pci_device_id *ent)
err_pci_disable:
pci_disable_device(pdev);
err_dev_put:
- drm_dev_put(dev);
+ drm_dev_put(&vbox->ddev);
return ret;
}
static void vbox_pci_remove(struct pci_dev *pdev)
{
- struct drm_device *dev = pci_get_drvdata(pdev);
- struct vbox_private *vbox = dev->dev_private;
+ struct vbox_private *vbox = pci_get_drvdata(pdev);
- drm_dev_unregister(dev);
- vbox_fbdev_fini(dev);
+ drm_dev_unregister(&vbox->ddev);
+ vbox_fbdev_fini(vbox);
vbox_irq_fini(vbox);
- vbox_mode_fini(dev);
+ vbox_mode_fini(vbox);
vbox_mm_fini(vbox);
vbox_hw_fini(vbox);
- drm_dev_put(dev);
+ drm_dev_put(&vbox->ddev);
}
-static int vbox_drm_freeze(struct drm_device *dev)
+static int vbox_drm_freeze(struct vbox_private *vbox)
{
- struct vbox_private *vbox = dev->dev_private;
-
- drm_kms_helper_poll_disable(dev);
+ drm_kms_helper_poll_disable(&vbox->ddev);
- pci_save_state(dev->pdev);
+ pci_save_state(vbox->ddev.pdev);
drm_fb_helper_set_suspend_unlocked(&vbox->fbdev->helper, true);
return 0;
}
-static int vbox_drm_thaw(struct drm_device *dev)
+static int vbox_drm_thaw(struct vbox_private *vbox)
{
- struct vbox_private *vbox = dev->dev_private;
-
- drm_mode_config_reset(dev);
- drm_helper_resume_force_mode(dev);
+ drm_mode_config_reset(&vbox->ddev);
+ drm_helper_resume_force_mode(&vbox->ddev);
drm_fb_helper_set_suspend_unlocked(&vbox->fbdev->helper, false);
return 0;
}
-static int vbox_drm_resume(struct drm_device *dev)
+static int vbox_drm_resume(struct vbox_private *vbox
[PATCH 11/12] staging: vboxvideo: Move bo_[un]resere calls into vbox_bo_[un]pin
We always need to reserve the bo around a pin / unpin, so move the
reservation there.
This allows removing the vbox_fb_[un]pin helpers.
Note this means that we now no longer hold the bo reserved while
k[un]mapping it in vbox_fb.c this is fine as it is not necessary
to hold it reserved for this.
Signed-off-by: Hans de Goede
---
drivers/staging/vboxvideo/vbox_drv.h | 2 +-
drivers/staging/vboxvideo/vbox_fb.c | 27 --
drivers/staging/vboxvideo/vbox_mode.c | 54 ---
drivers/staging/vboxvideo/vbox_ttm.c | 32 +---
4 files changed, 42 insertions(+), 73 deletions(-)
diff --git a/drivers/staging/vboxvideo/vbox_drv.h
b/drivers/staging/vboxvideo/vbox_drv.h
index 6a4d3b382e79..fffde1713101 100644
--- a/drivers/staging/vboxvideo/vbox_drv.h
+++ b/drivers/staging/vboxvideo/vbox_drv.h
@@ -245,7 +245,7 @@ int vbox_bo_create(struct vbox_private *vbox, int size, int
align,
int vbox_gem_create(struct vbox_private *vbox,
u32 size, bool iskernel, struct drm_gem_object **obj);
-int vbox_bo_pin(struct vbox_bo *bo, u32 pl_flag, u64 *gpu_addr);
+int vbox_bo_pin(struct vbox_bo *bo, u32 pl_flag);
int vbox_bo_unpin(struct vbox_bo *bo);
static inline int vbox_bo_reserve(struct vbox_bo *bo, bool no_wait)
diff --git a/drivers/staging/vboxvideo/vbox_fb.c
b/drivers/staging/vboxvideo/vbox_fb.c
index 0e5550fa7c57..bdc87d02ecc5 100644
--- a/drivers/staging/vboxvideo/vbox_fb.c
+++ b/drivers/staging/vboxvideo/vbox_fb.c
@@ -104,18 +104,11 @@ static int vboxfb_create(struct drm_fb_helper *helper,
bo = gem_to_vbox_bo(gobj);
- ret = vbox_bo_reserve(bo, false);
+ ret = vbox_bo_pin(bo, TTM_PL_FLAG_VRAM);
if (ret)
return ret;
- ret = vbox_bo_pin(bo, TTM_PL_FLAG_VRAM, &gpu_addr);
- if (ret) {
- vbox_bo_unreserve(bo);
- return ret;
- }
-
ret = ttm_bo_kmap(&bo->bo, 0, bo->bo.num_pages, &bo->kmap);
- vbox_bo_unreserve(bo);
if (ret) {
DRM_ERROR("failed to kmap fbcon\n");
return ret;
@@ -153,6 +146,7 @@ static int vboxfb_create(struct drm_fb_helper *helper,
drm_fb_helper_fill_var(info, &fbdev->helper, sizes->fb_width,
sizes->fb_height);
+ gpu_addr = vbox_bo_gpu_offset(bo);
info->fix.smem_start = info->apertures->ranges[0].base + gpu_addr;
info->fix.smem_len = vbox->available_vram_size - gpu_addr;
@@ -190,17 +184,12 @@ void vbox_fbdev_fini(struct vbox_private *vbox)
if (afb->obj) {
struct vbox_bo *bo = gem_to_vbox_bo(afb->obj);
- if (!vbox_bo_reserve(bo, false)) {
- if (bo->kmap.virtual)
- ttm_bo_kunmap(&bo->kmap);
- /*
-* QXL does this, but is it really needed before
-* freeing?
-*/
- if (bo->pin_count)
- vbox_bo_unpin(bo);
- vbox_bo_unreserve(bo);
- }
+ if (bo->kmap.virtual)
+ ttm_bo_kunmap(&bo->kmap);
+
+ if (bo->pin_count)
+ vbox_bo_unpin(bo);
+
drm_gem_object_put_unlocked(afb->obj);
afb->obj = NULL;
}
diff --git a/drivers/staging/vboxvideo/vbox_mode.c
b/drivers/staging/vboxvideo/vbox_mode.c
index 910ea19931c9..bef99664d030 100644
--- a/drivers/staging/vboxvideo/vbox_mode.c
+++ b/drivers/staging/vboxvideo/vbox_mode.c
@@ -221,40 +221,6 @@ static bool vbox_set_up_input_mapping(struct vbox_private
*vbox)
return old_single_framebuffer != vbox->single_framebuffer;
}
-static int vbox_fb_pin(struct drm_framebuffer *fb, u32 pl_flag, u64 *addr)
-{
- struct vbox_bo *bo = gem_to_vbox_bo(to_vbox_framebuffer(fb)->obj);
- int ret;
-
- ret = vbox_bo_reserve(bo, false);
- if (ret)
- return ret;
-
- ret = vbox_bo_pin(bo, pl_flag, addr);
- vbox_bo_unreserve(bo);
- return ret;
-}
-
-static void vbox_fb_unpin(struct drm_framebuffer *fb)
-{
- struct vbox_bo *bo;
- int ret;
-
- if (!fb)
- return;
-
- bo = gem_to_vbox_bo(to_vbox_framebuffer(fb)->obj);
-
- ret = vbox_bo_reserve(bo, false);
- if (ret) {
- DRM_ERROR("Error %d reserving fb bo, leaving it pinned\n", ret);
- return;
- }
-
- vbox_bo_unpin(bo);
- vbox_bo_unreserve(bo);
-}
-
static void vbox_crtc_set_base_and_mode(struct drm_crtc *crtc,
struct drm_framebuffer *fb,
struct drm_display_mode *mode,
@@ -296,17 +262,22 @@ static int vbox_crtc_mode_set(struct drm_crtc *crtc,
struct drm_display_mode *adjusted_mode,
int x, int y, struct drm_framebuff
[PATCH 12/12] staging: vboxvideo: Add vbox_bo_k[un]map helper functions
Add vbox_bo_k[un]map helper functions instead of having code directly
poking struct vbox_bo internals.
While touch neighboring code anyways also fix a return -PTR_ERR(info),
which should be return PTR_ERR(info).
Signed-off-by: Hans de Goede
---
drivers/staging/vboxvideo/vbox_drv.h | 2 ++
drivers/staging/vboxvideo/vbox_fb.c | 19 +++
drivers/staging/vboxvideo/vbox_ttm.c | 28 +++-
3 files changed, 36 insertions(+), 13 deletions(-)
diff --git a/drivers/staging/vboxvideo/vbox_drv.h
b/drivers/staging/vboxvideo/vbox_drv.h
index fffde1713101..6c52cbd9e91e 100644
--- a/drivers/staging/vboxvideo/vbox_drv.h
+++ b/drivers/staging/vboxvideo/vbox_drv.h
@@ -269,6 +269,8 @@ static inline void vbox_bo_unreserve(struct vbox_bo *bo)
void vbox_ttm_placement(struct vbox_bo *bo, int domain);
int vbox_bo_push_sysram(struct vbox_bo *bo);
int vbox_mmap(struct file *filp, struct vm_area_struct *vma);
+void *vbox_bo_kmap(struct vbox_bo *bo);
+void vbox_bo_kunmap(struct vbox_bo *bo);
/* vbox_prime.c */
int vbox_gem_prime_pin(struct drm_gem_object *obj);
diff --git a/drivers/staging/vboxvideo/vbox_fb.c
b/drivers/staging/vboxvideo/vbox_fb.c
index bdc87d02ecc5..b8b42f9aafae 100644
--- a/drivers/staging/vboxvideo/vbox_fb.c
+++ b/drivers/staging/vboxvideo/vbox_fb.c
@@ -108,15 +108,14 @@ static int vboxfb_create(struct drm_fb_helper *helper,
if (ret)
return ret;
- ret = ttm_bo_kmap(&bo->bo, 0, bo->bo.num_pages, &bo->kmap);
- if (ret) {
- DRM_ERROR("failed to kmap fbcon\n");
- return ret;
- }
-
info = drm_fb_helper_alloc_fbi(helper);
if (IS_ERR(info))
- return -PTR_ERR(info);
+ return PTR_ERR(info);
+
+ info->screen_size = size;
+ info->screen_base = (char __iomem *)vbox_bo_kmap(bo);
+ if (IS_ERR(info->screen_base))
+ return PTR_ERR(info->screen_base);
info->par = fbdev;
@@ -150,9 +149,6 @@ static int vboxfb_create(struct drm_fb_helper *helper,
info->fix.smem_start = info->apertures->ranges[0].base + gpu_addr;
info->fix.smem_len = vbox->available_vram_size - gpu_addr;
- info->screen_base = (char __iomem *)bo->kmap.virtual;
- info->screen_size = size;
-
#ifdef CONFIG_DRM_KMS_FB_HELPER
info->fbdefio = &vbox_defio;
fb_deferred_io_init(info);
@@ -184,8 +180,7 @@ void vbox_fbdev_fini(struct vbox_private *vbox)
if (afb->obj) {
struct vbox_bo *bo = gem_to_vbox_bo(afb->obj);
- if (bo->kmap.virtual)
- ttm_bo_kunmap(&bo->kmap);
+ vbox_bo_kunmap(bo);
if (bo->pin_count)
vbox_bo_unpin(bo);
diff --git a/drivers/staging/vboxvideo/vbox_ttm.c
b/drivers/staging/vboxvideo/vbox_ttm.c
index bd0a1603764e..5ecfa7629173 100644
--- a/drivers/staging/vboxvideo/vbox_ttm.c
+++ b/drivers/staging/vboxvideo/vbox_ttm.c
@@ -418,8 +418,10 @@ int vbox_bo_push_sysram(struct vbox_bo *bo)
if (bo->pin_count)
return 0;
- if (bo->kmap.virtual)
+ if (bo->kmap.virtual) {
ttm_bo_kunmap(&bo->kmap);
+ bo->kmap.virtual = NULL;
+ }
vbox_ttm_placement(bo, TTM_PL_FLAG_SYSTEM);
@@ -448,3 +450,27 @@ int vbox_mmap(struct file *filp, struct vm_area_struct
*vma)
return ttm_bo_mmap(filp, vma, &vbox->ttm.bdev);
}
+
+void *vbox_bo_kmap(struct vbox_bo *bo)
+{
+ int ret;
+
+ if (bo->kmap.virtual)
+ return bo->kmap.virtual;
+
+ ret = ttm_bo_kmap(&bo->bo, 0, bo->bo.num_pages, &bo->kmap);
+ if (ret) {
+ DRM_ERROR("Error kmapping bo: %d\n", ret);
+ return NULL;
+ }
+
+ return bo->kmap.virtual;
+}
+
+void vbox_bo_kunmap(struct vbox_bo *bo)
+{
+ if (bo->kmap.virtual) {
+ ttm_bo_kunmap(&bo->kmap);
+ bo->kmap.virtual = NULL;
+ }
+}
--
2.19.0.rc1
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH v3] staging: android: ion: Add per-heap counters
On Tue, Sep 11, 2018 at 02:29:19PM +0300, Alexey Skidanov wrote:
> Heap statistics have been removed and currently even basics statistics
> are missing.
>
> This patch creates per heap debugfs directory /sys/kernel/debug/
> and adds the following counters:
> - the number of allocated buffers;
> - the number of allocated bytes;
> - the number of allocated bytes watermark.
>
> Signed-off-by: Alexey Skidanov
> ---
It would be great to get an ack from some of the ion developers...
{hint}
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH v2 05/17] compat_ioctl: move more drivers to generic_compat_ioctl_ptrarg
On Fri, Sep 14, 2018 at 09:57:48PM +0100, Al Viro wrote: > On Fri, Sep 14, 2018 at 01:35:06PM -0700, Darren Hart wrote: > > > Acked-by: Darren Hart (VMware) > > > > As for a longer term solution, would it be possible to init fops in such > > a way that the compat_ioctl call defaults to generic_compat_ioctl_ptrarg > > so we don't have to duplicate this boilerplate for every ioctl fops > > structure? > > Bad idea, that... Because several years down the road somebody will add > an ioctl that takes an unsigned int for argument. Without so much as looking > at your magical mystery macro being used to initialize file_operations. Fair, being explicit in the declaration as it is currently may be preferable then. -- Darren Hart VMware Open Source Technology Center ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH v2 05/17] compat_ioctl: move more drivers to generic_compat_ioctl_ptrarg
On Tue, Sep 18, 2018 at 10:51:08AM -0700, Darren Hart wrote: > On Fri, Sep 14, 2018 at 09:57:48PM +0100, Al Viro wrote: > > On Fri, Sep 14, 2018 at 01:35:06PM -0700, Darren Hart wrote: > > > > > Acked-by: Darren Hart (VMware) > > > > > > As for a longer term solution, would it be possible to init fops in such > > > a way that the compat_ioctl call defaults to generic_compat_ioctl_ptrarg > > > so we don't have to duplicate this boilerplate for every ioctl fops > > > structure? > > > > Bad idea, that... Because several years down the road somebody will add > > an ioctl that takes an unsigned int for argument. Without so much as > > looking > > at your magical mystery macro being used to initialize file_operations. > > Fair, being explicit in the declaration as it is currently may be > preferable then. It would be much cleaner and safer if you could arrange things to add something like this to struct file_operations: long (*ptr_ioctl) (struct file *, unsigned int, void __user *); Where the core code automatically converts the unsigned long to the void __user * as appropriate. Then it just works right always and the compiler will help address Al's concern down the road. Cheers, Jason ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH v1 1/6] mm/memory_hotplug: make remove_memory() take the device_hotplug_lock
On Tue, Sep 18, 2018 at 1:48 PM David Hildenbrand wrote:
>
> remove_memory() is exported right now but requires the
> device_hotplug_lock, which is not exported. So let's provide a variant
> that takes the lock and only export that one.
>
> The lock is already held in
> arch/powerpc/platforms/pseries/hotplug-memory.c
> drivers/acpi/acpi_memhotplug.c
> So, let's use the locked variant.
>
> The lock is not held (but taken in)
> arch/powerpc/platforms/powernv/memtrace.c
> So let's keep using the (now) locked variant.
>
> Apart from that, there are not other users in the tree.
>
> Cc: Benjamin Herrenschmidt
> Cc: Paul Mackerras
> Cc: Michael Ellerman
> Cc: "Rafael J. Wysocki"
> Cc: Len Brown
> Cc: Rashmica Gupta
> Cc: Michael Neuling
> Cc: Balbir Singh
> Cc: Nathan Fontenot
> Cc: John Allen
> Cc: Andrew Morton
> Cc: Michal Hocko
> Cc: Dan Williams
> Cc: Joonsoo Kim
> Cc: Vlastimil Babka
> Cc: Pavel Tatashin
> Cc: Greg Kroah-Hartman
> Cc: Oscar Salvador
> Cc: YASUAKI ISHIMATSU
> Cc: Mathieu Malaterre
> Reviewed-by: Pavel Tatashin
> Signed-off-by: David Hildenbrand
> ---
> arch/powerpc/platforms/powernv/memtrace.c | 2 --
> arch/powerpc/platforms/pseries/hotplug-memory.c | 6 +++---
> drivers/acpi/acpi_memhotplug.c | 2 +-
> include/linux/memory_hotplug.h | 3 ++-
> mm/memory_hotplug.c | 9 -
> 5 files changed, 14 insertions(+), 8 deletions(-)
>
> diff --git a/arch/powerpc/platforms/powernv/memtrace.c
> b/arch/powerpc/platforms/powernv/memtrace.c
> index 51dc398ae3f7..8f1cd4f3bfd5 100644
> --- a/arch/powerpc/platforms/powernv/memtrace.c
> +++ b/arch/powerpc/platforms/powernv/memtrace.c
> @@ -90,9 +90,7 @@ static bool memtrace_offline_pages(u32 nid, u64 start_pfn,
> u64 nr_pages)
> walk_memory_range(start_pfn, end_pfn, (void *)MEM_OFFLINE,
> change_memblock_state);
>
> - lock_device_hotplug();
> remove_memory(nid, start_pfn << PAGE_SHIFT, nr_pages << PAGE_SHIFT);
> - unlock_device_hotplug();
>
> return true;
> }
> diff --git a/arch/powerpc/platforms/pseries/hotplug-memory.c
> b/arch/powerpc/platforms/pseries/hotplug-memory.c
> index c1578f54c626..b3f54466e25f 100644
> --- a/arch/powerpc/platforms/pseries/hotplug-memory.c
> +++ b/arch/powerpc/platforms/pseries/hotplug-memory.c
> @@ -334,7 +334,7 @@ static int pseries_remove_memblock(unsigned long base,
> unsigned int memblock_siz
> nid = memory_add_physaddr_to_nid(base);
>
> for (i = 0; i < sections_per_block; i++) {
> - remove_memory(nid, base, MIN_MEMORY_BLOCK_SIZE);
> + __remove_memory(nid, base, MIN_MEMORY_BLOCK_SIZE);
> base += MIN_MEMORY_BLOCK_SIZE;
> }
>
> @@ -423,7 +423,7 @@ static int dlpar_remove_lmb(struct drmem_lmb *lmb)
> block_sz = pseries_memory_block_size();
> nid = memory_add_physaddr_to_nid(lmb->base_addr);
>
> - remove_memory(nid, lmb->base_addr, block_sz);
> + __remove_memory(nid, lmb->base_addr, block_sz);
>
> /* Update memory regions for memory remove */
> memblock_remove(lmb->base_addr, block_sz);
> @@ -710,7 +710,7 @@ static int dlpar_add_lmb(struct drmem_lmb *lmb)
>
> rc = dlpar_online_lmb(lmb);
> if (rc) {
> - remove_memory(nid, lmb->base_addr, block_sz);
> + __remove_memory(nid, lmb->base_addr, block_sz);
> dlpar_remove_device_tree_lmb(lmb);
> } else {
> lmb->flags |= DRCONF_MEM_ASSIGNED;
> diff --git a/drivers/acpi/acpi_memhotplug.c b/drivers/acpi/acpi_memhotplug.c
> index 6b0d3ef7309c..811148415993 100644
> --- a/drivers/acpi/acpi_memhotplug.c
> +++ b/drivers/acpi/acpi_memhotplug.c
> @@ -282,7 +282,7 @@ static void acpi_memory_remove_memory(struct
> acpi_memory_device *mem_device)
> nid = memory_add_physaddr_to_nid(info->start_addr);
>
> acpi_unbind_memory_blocks(info);
> - remove_memory(nid, info->start_addr, info->length);
> + __remove_memory(nid, info->start_addr, info->length);
> list_del(&info->list);
> kfree(info);
> }
> diff --git a/include/linux/memory_hotplug.h b/include/linux/memory_hotplug.h
> index 34a28227068d..1f096852f479 100644
> --- a/include/linux/memory_hotplug.h
> +++ b/include/linux/memory_hotplug.h
> @@ -301,6 +301,7 @@ extern bool is_mem_section_removable(unsigned long pfn,
> unsigned long nr_pages);
> extern void try_offline_node(int nid);
> extern int offline_pages(unsigned long start_pfn, unsigned long nr_pages);
> extern void remove_memory(int nid, u64 start, u64 size);
> +extern void __remove_memory(int nid, u64 start, u64 size);
>
> #else
> static inline bool is_mem_section_removable(unsigned long pfn,
> @@ -317,6 +318,7 @@ static inline int offline_pages(unsigned long start_pfn,
> unsigned long nr_pages)
> }
>
>
Re: [PATCH v1 2/6] mm/memory_hotplug: make add_memory() take the device_hotplug_lock
On Tue, Sep 18, 2018 at 1:48 PM David Hildenbrand wrote:
>
> add_memory() currently does not take the device_hotplug_lock, however
> is aleady called under the lock from
> arch/powerpc/platforms/pseries/hotplug-memory.c
> drivers/acpi/acpi_memhotplug.c
> to synchronize against CPU hot-remove and similar.
>
> In general, we should hold the device_hotplug_lock when adding memory
> to synchronize against online/offline request (e.g. from user space) -
> which already resulted in lock inversions due to device_lock() and
> mem_hotplug_lock - see 30467e0b3be ("mm, hotplug: fix concurrent memory
> hot-add deadlock"). add_memory()/add_memory_resource() will create memory
> block devices, so this really feels like the right thing to do.
>
> Holding the device_hotplug_lock makes sure that a memory block device
> can really only be accessed (e.g. via .online/.state) from user space,
> once the memory has been fully added to the system.
>
> The lock is not held yet in
> drivers/xen/balloon.c
> arch/powerpc/platforms/powernv/memtrace.c
> drivers/s390/char/sclp_cmd.c
> drivers/hv/hv_balloon.c
> So, let's either use the locked variants or take the lock.
>
> Don't export add_memory_resource(), as it once was exported to be used
> by XEN, which is never built as a module. If somebody requires it, we
> also have to export a locked variant (as device_hotplug_lock is never
> exported).
>
> Cc: Benjamin Herrenschmidt
> Cc: Paul Mackerras
> Cc: Michael Ellerman
> Cc: "Rafael J. Wysocki"
> Cc: Len Brown
> Cc: Greg Kroah-Hartman
> Cc: Boris Ostrovsky
> Cc: Juergen Gross
> Cc: Nathan Fontenot
> Cc: John Allen
> Cc: Andrew Morton
> Cc: Michal Hocko
> Cc: Dan Williams
> Cc: Joonsoo Kim
> Cc: Vlastimil Babka
> Cc: Oscar Salvador
> Cc: Mathieu Malaterre
> Cc: Pavel Tatashin
> Cc: YASUAKI ISHIMATSU
> Reviewed-by: Pavel Tatashin
> Signed-off-by: David Hildenbrand
> ---
> .../platforms/pseries/hotplug-memory.c| 2 +-
> drivers/acpi/acpi_memhotplug.c| 2 +-
> drivers/base/memory.c | 9 ++--
> drivers/xen/balloon.c | 3 +++
> include/linux/memory_hotplug.h| 1 +
> mm/memory_hotplug.c | 22 ---
> 6 files changed, 32 insertions(+), 7 deletions(-)
>
> diff --git a/arch/powerpc/platforms/pseries/hotplug-memory.c
> b/arch/powerpc/platforms/pseries/hotplug-memory.c
> index b3f54466e25f..2e6f41dc103a 100644
> --- a/arch/powerpc/platforms/pseries/hotplug-memory.c
> +++ b/arch/powerpc/platforms/pseries/hotplug-memory.c
> @@ -702,7 +702,7 @@ static int dlpar_add_lmb(struct drmem_lmb *lmb)
> nid = memory_add_physaddr_to_nid(lmb->base_addr);
>
> /* Add the memory */
> - rc = add_memory(nid, lmb->base_addr, block_sz);
> + rc = __add_memory(nid, lmb->base_addr, block_sz);
> if (rc) {
> dlpar_remove_device_tree_lmb(lmb);
> return rc;
> diff --git a/drivers/acpi/acpi_memhotplug.c b/drivers/acpi/acpi_memhotplug.c
> index 811148415993..8fe0960ea572 100644
> --- a/drivers/acpi/acpi_memhotplug.c
> +++ b/drivers/acpi/acpi_memhotplug.c
> @@ -228,7 +228,7 @@ static int acpi_memory_enable_device(struct
> acpi_memory_device *mem_device)
> if (node < 0)
> node = memory_add_physaddr_to_nid(info->start_addr);
>
> - result = add_memory(node, info->start_addr, info->length);
> + result = __add_memory(node, info->start_addr, info->length);
>
> /*
> * If the memory block has been used by the kernel,
> add_memory()
> diff --git a/drivers/base/memory.c b/drivers/base/memory.c
> index 817320c7c4c1..40cac122ec73 100644
> --- a/drivers/base/memory.c
> +++ b/drivers/base/memory.c
> @@ -519,15 +519,20 @@ memory_probe_store(struct device *dev, struct
> device_attribute *attr,
> if (phys_addr & ((pages_per_block << PAGE_SHIFT) - 1))
> return -EINVAL;
>
> + ret = lock_device_hotplug_sysfs();
> + if (ret)
> + goto out;
> +
> nid = memory_add_physaddr_to_nid(phys_addr);
> - ret = add_memory(nid, phys_addr,
> -MIN_MEMORY_BLOCK_SIZE * sections_per_block);
> + ret = __add_memory(nid, phys_addr,
> + MIN_MEMORY_BLOCK_SIZE * sections_per_block);
>
> if (ret)
> goto out;
>
> ret = count;
> out:
> + unlock_device_hotplug();
> return ret;
> }
>
> diff --git a/drivers/xen/balloon.c b/drivers/xen/balloon.c
> index e12bb256036f..6bab019a82b1 100644
> --- a/drivers/xen/balloon.c
> +++ b/drivers/xen/balloon.c
> @@ -395,7 +395,10 @@ static enum bp_state reserve_additional_memory(void)
> * callers drop the mutex before trying again.
> */
> mutex_unlock(&balloon_mutex);
> + /* add_memory_resource() requires the device_hotplug lo
[PATCH] x86/hyperv: suppress "PCI: Fatal: No config space access function found"
A Generatin-2 Linux VM on Hyper-V doesn't have the legacy PCI bus, and
users always see the scary warning, which is actually harmless. The patch
is made to suppress the warning.
Signed-off-by: Dexuan Cui
Cc: K. Y. Srinivasan
Cc: Haiyang Zhang
Cc: Stephen Hemminger
---
arch/x86/hyperv/hv_init.c | 19 +++
1 file changed, 19 insertions(+)
diff --git a/arch/x86/hyperv/hv_init.c b/arch/x86/hyperv/hv_init.c
index 20c876c..7abb09e 100644
--- a/arch/x86/hyperv/hv_init.c
+++ b/arch/x86/hyperv/hv_init.c
@@ -17,6 +17,7 @@
*
*/
+#include
#include
#include
#include
@@ -253,6 +254,22 @@ static int hv_cpu_die(unsigned int cpu)
return 0;
}
+static int __init hv_pci_init(void)
+{
+ int gen2vm = efi_enabled(EFI_BOOT);
+
+ /*
+* For Generation-2 VM, we exit from pci_arch_init() by returning 0.
+* The purpose is to suppress the harmless warning:
+* "PCI: Fatal: No config space access function found"
+*/
+ if (gen2vm)
+ return 0;
+
+ /* For Generation-1 VM, we'll proceed in pci_arch_init(). */
+ return 1;
+}
+
/*
* This function is to be invoked early in the boot sequence after the
* hypervisor has been detected.
@@ -329,6 +346,8 @@ void __init hyperv_init(void)
hv_apic_init();
+ x86_init.pci.arch_init = hv_pci_init;
+
/*
* Register Hyper-V specific clocksource.
*/
--
2.7.4
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [patch 09/11] x86/vdso: Simplify the invalid vclock case
On Tue, 18 Sep 2018, Andy Lutomirski wrote: > > On Sep 18, 2018, at 12:52 AM, Thomas Gleixner wrote: > > > >> On Mon, 17 Sep 2018, John Stultz wrote: > >>> On Mon, Sep 17, 2018 at 12:25 PM, Andy Lutomirski wrote: > >>> Also, I'm not entirely convinced that this "last" thing is needed at > >>> all. John, what's the scenario under which we need it? > >> > >> So my memory is probably a bit foggy, but I recall that as we > >> accelerated gettimeofday, we found that even on systems that claimed > >> to have synced TSCs, they were actually just slightly out of sync. > >> Enough that right after cycles_last had been updated, a read on > >> another cpu could come in just behind cycles_last, resulting in a > >> negative interval causing lots of havoc. > >> > >> So the sanity check is needed to avoid that case. > > > > Your memory serves you right. That's indeed observable on CPUs which > > lack TSC_ADJUST. > > > > @Andy: Welcome to the wonderful world of TSC. > > > > Do we do better if we use signed arithmetic for the whole calculation? > Then a small backwards movement would result in a small backwards result. > Or we could offset everything so that we’d have to go back several > hundred ms before we cross zero. That would be probably the better solution as signed math would be problematic when the resulting ns value becomes negative. As the delta is really small, otherwise the TSC sync check would have caught it, the caller should never be able to observe time going backwards. I'll have a look into that. It needs some thought vs. the fractional part of the base time, but it should be not rocket science to get that correct. Famous last words... Thanks, tglx ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [patch 09/11] x86/vdso: Simplify the invalid vclock case
> On Sep 18, 2018, at 3:46 PM, Thomas Gleixner wrote: > > On Tue, 18 Sep 2018, Andy Lutomirski wrote: >>> On Sep 18, 2018, at 12:52 AM, Thomas Gleixner wrote: >>> > On Mon, 17 Sep 2018, John Stultz wrote: > On Mon, Sep 17, 2018 at 12:25 PM, Andy Lutomirski wrote: > Also, I'm not entirely convinced that this "last" thing is needed at > all. John, what's the scenario under which we need it? So my memory is probably a bit foggy, but I recall that as we accelerated gettimeofday, we found that even on systems that claimed to have synced TSCs, they were actually just slightly out of sync. Enough that right after cycles_last had been updated, a read on another cpu could come in just behind cycles_last, resulting in a negative interval causing lots of havoc. So the sanity check is needed to avoid that case. >>> >>> Your memory serves you right. That's indeed observable on CPUs which >>> lack TSC_ADJUST. >>> >>> @Andy: Welcome to the wonderful world of TSC. >>> >> >> Do we do better if we use signed arithmetic for the whole calculation? >> Then a small backwards movement would result in a small backwards result. >> Or we could offset everything so that we’d have to go back several >> hundred ms before we cross zero. > > That would be probably the better solution as signed math would be > problematic when the resulting ns value becomes negative. As the delta is > really small, otherwise the TSC sync check would have caught it, the caller > should never be able to observe time going backwards. > > I'll have a look into that. It needs some thought vs. the fractional part > of the base time, but it should be not rocket science to get that > correct. Famous last words... > It’s also fiddly to tune. If you offset it too much, then the fancy divide-by-repeated-subtraction loop will hurt more than the comparison to last. ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [patch 09/11] x86/vdso: Simplify the invalid vclock case
On Tue, 18 Sep 2018, Andy Lutomirski wrote: > > On Sep 18, 2018, at 3:46 PM, Thomas Gleixner wrote: > > On Tue, 18 Sep 2018, Andy Lutomirski wrote: > >> Do we do better if we use signed arithmetic for the whole calculation? > >> Then a small backwards movement would result in a small backwards result. > >> Or we could offset everything so that we’d have to go back several > >> hundred ms before we cross zero. > > > > That would be probably the better solution as signed math would be > > problematic when the resulting ns value becomes negative. As the delta is > > really small, otherwise the TSC sync check would have caught it, the caller > > should never be able to observe time going backwards. > > > > I'll have a look into that. It needs some thought vs. the fractional part > > of the base time, but it should be not rocket science to get that > > correct. Famous last words... > > > > It’s also fiddly to tune. If you offset it too much, then the fancy > divide-by-repeated-subtraction loop will hurt more than the comparison to > last. Not really. It's sufficient to offset it by at max. 1000 cycles or so. That won't hurt the magic loop, but it will definitely cover that slight offset case. Thanks, tglx ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH v1 0/6] mm: online/offline_pages called w.o. mem_hotplug_lock
On Tue, Sep 18, 2018 at 01:48:16PM +0200, David Hildenbrand wrote: > Reading through the code and studying how mem_hotplug_lock is to be used, > I noticed that there are two places where we can end up calling > device_online()/device_offline() - online_pages()/offline_pages() without > the mem_hotplug_lock. And there are other places where we call > device_online()/device_offline() without the device_hotplug_lock. > > While e.g. > echo "online" > /sys/devices/system/memory/memory9/state > is fine, e.g. > echo 1 > /sys/devices/system/memory/memory9/online > Will not take the mem_hotplug_lock. However the device_lock() and > device_hotplug_lock. > > E.g. via memory_probe_store(), we can end up calling > add_memory()->online_pages() without the device_hotplug_lock. So we can > have concurrent callers in online_pages(). We e.g. touch in online_pages() > basically unprotected zone->present_pages then. > > Looks like there is a longer history to that (see Patch #2 for details), > and fixing it to work the way it was intended is not really possible. We > would e.g. have to take the mem_hotplug_lock in device/base/core.c, which > sounds wrong. > > Summary: We had a lock inversion on mem_hotplug_lock and device_lock(). > More details can be found in patch 3 and patch 6. > > I propose the general rules (documentation added in patch 6): > > 1. add_memory/add_memory_resource() must only be called with >device_hotplug_lock. > 2. remove_memory() must only be called with device_hotplug_lock. This is >already documented and holds for all callers. > 3. device_online()/device_offline() must only be called with >device_hotplug_lock. This is already documented and true for now in core >code. Other callers (related to memory hotplug) have to be fixed up. > 4. mem_hotplug_lock is taken inside of add_memory/remove_memory/ >online_pages/offline_pages. > > To me, this looks way cleaner than what we have right now (and easier to > verify). And looking at the documentation of remove_memory, using > lock_device_hotplug also for add_memory() feels natural. > That seems reasonable, but also implies that device_online() would hold back add/remove memory, could you please also document what mode read/write the locks need to be held? For example can the device_hotplug_lock be held in read mode while add/remove memory via (mem_hotplug_lock) is held in write mode? Balbir Singh. ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH 2/6] staging: erofs: fold in `__update_workgrp_llen'
There is the only one user to use `__update_workgrp_llen'.
Fold it in `z_erofs_vle_work_iter_begin' and cleanup related code.
Signed-off-by: Gao Xiang
---
drivers/staging/erofs/unzip_vle.c | 20 +++-
1 file changed, 7 insertions(+), 13 deletions(-)
diff --git a/drivers/staging/erofs/unzip_vle.c
b/drivers/staging/erofs/unzip_vle.c
index 2b9b313..e820490 100644
--- a/drivers/staging/erofs/unzip_vle.c
+++ b/drivers/staging/erofs/unzip_vle.c
@@ -422,18 +422,6 @@ struct z_erofs_vle_work_finder {
return work;
}
-static inline void __update_workgrp_llen(struct z_erofs_vle_workgroup *grp,
-unsigned int llen)
-{
- while (1) {
- unsigned int orig_llen = grp->llen;
-
- if (orig_llen >= llen || orig_llen ==
- cmpxchg(&grp->llen, orig_llen, llen))
- break;
- }
-}
-
#define builder_is_followed(builder) \
((builder)->role >= Z_EROFS_VLE_WORK_PRIMARY_FOLLOWED)
@@ -466,7 +454,13 @@ static int z_erofs_vle_work_iter_begin(struct
z_erofs_vle_work_builder *builder,
repeat:
work = z_erofs_vle_work_lookup(&finder);
if (work != NULL) {
- __update_workgrp_llen(grp, map->m_llen);
+ unsigned int orig_llen;
+
+ /* increase workgroup `llen' if needed */
+ while ((orig_llen = READ_ONCE(grp->llen)) < map->m_llen &&
+ orig_llen != cmpxchg_relaxed(&grp->llen,
+ orig_llen, map->m_llen))
+ cpu_relax();
goto got_it;
}
--
1.9.1
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH 1/6] staging: erofs: remove redundant CONFIG_EROFS_FS_XATTRs
some CONFIG_EROFS_FS_XATTR conditions were added because of
the historial Linux kernel compatibility, which are unneeded now.
Signed-off-by: Gao Xiang
---
Hi all,
These are cleanup patches in Chao's erofs-dev test tree for a while.
Many of them are trivial.
In order for all preview patches to keep up with the staging tree,
I resend them now.
Thanks,
Gao Xiang
drivers/staging/erofs/inode.c| 6 --
drivers/staging/erofs/internal.h | 6 ++
2 files changed, 2 insertions(+), 10 deletions(-)
diff --git a/drivers/staging/erofs/inode.c b/drivers/staging/erofs/inode.c
index c46a8d4..da8693a 100644
--- a/drivers/staging/erofs/inode.c
+++ b/drivers/staging/erofs/inode.c
@@ -260,22 +260,16 @@ struct inode *erofs_iget(struct super_block *sb,
const struct inode_operations erofs_generic_xattr_iops = {
.listxattr = erofs_listxattr,
};
-#endif
-#ifdef CONFIG_EROFS_FS_XATTR
const struct inode_operations erofs_symlink_xattr_iops = {
.get_link = page_get_link,
.listxattr = erofs_listxattr,
};
-#endif
const struct inode_operations erofs_special_inode_operations = {
-#ifdef CONFIG_EROFS_FS_XATTR
.listxattr = erofs_listxattr,
-#endif
};
-#ifdef CONFIG_EROFS_FS_XATTR
const struct inode_operations erofs_fast_symlink_xattr_iops = {
.get_link = simple_get_link,
.listxattr = erofs_listxattr,
diff --git a/drivers/staging/erofs/internal.h b/drivers/staging/erofs/internal.h
index 0011b9d..cfcc6db 100644
--- a/drivers/staging/erofs/internal.h
+++ b/drivers/staging/erofs/internal.h
@@ -507,13 +507,11 @@ extern struct inode *erofs_iget(struct super_block *sb,
int erofs_namei(struct inode *dir, struct qstr *name,
erofs_nid_t *nid, unsigned *d_type);
-/* xattr.c */
#ifdef CONFIG_EROFS_FS_XATTR
+/* xattr.c */
extern const struct xattr_handler *erofs_xattr_handlers[];
-#endif
-/* symlink */
-#ifdef CONFIG_EROFS_FS_XATTR
+/* symlink and special inode */
extern const struct inode_operations erofs_symlink_xattr_iops;
extern const struct inode_operations erofs_fast_symlink_xattr_iops;
extern const struct inode_operations erofs_special_inode_operations;
--
1.9.1
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH 3/6] staging: erofs: drop multiref support temporarily
Multiref support means that a compressed page could have
more than one reference, which is designed for on-disk data
deduplication. However, mkfs doesn't support this mode
at this moment, and the kernel implementation is also broken.
Let's drop multiref support. If it is fully implemented
in the future, it can be reverted later.
Signed-off-by: Gao Xiang
---
drivers/staging/erofs/unzip_vle.c | 36 +---
drivers/staging/erofs/unzip_vle.h | 12 +---
2 files changed, 6 insertions(+), 42 deletions(-)
diff --git a/drivers/staging/erofs/unzip_vle.c
b/drivers/staging/erofs/unzip_vle.c
index e820490..9fe18cd3 100644
--- a/drivers/staging/erofs/unzip_vle.c
+++ b/drivers/staging/erofs/unzip_vle.c
@@ -301,12 +301,9 @@ struct z_erofs_vle_work_finder {
grp = container_of(egrp, struct z_erofs_vle_workgroup, obj);
*f->grp_ret = grp;
-#ifndef CONFIG_EROFS_FS_ZIP_MULTIREF
work = z_erofs_vle_grab_work(grp, f->pageofs);
+ /* if multiref is disabled, `primary' is always true */
primary = true;
-#else
- BUG();
-#endif
DBG_BUGON(work->pageofs != f->pageofs);
@@ -368,12 +365,9 @@ struct z_erofs_vle_work_finder {
struct z_erofs_vle_workgroup *grp = *f->grp_ret;
struct z_erofs_vle_work *work;
-#ifndef CONFIG_EROFS_FS_ZIP_MULTIREF
+ /* if multiref is disabled, grp should never be nullptr */
BUG_ON(grp != NULL);
-#else
- if (grp != NULL)
- goto skip;
-#endif
+
/* no available workgroup, let's allocate one */
grp = kmem_cache_zalloc(z_erofs_workgroup_cachep, GFP_NOFS);
if (unlikely(grp == NULL))
@@ -396,13 +390,7 @@ struct z_erofs_vle_work_finder {
*f->hosted = true;
gnew = true;
-#ifdef CONFIG_EROFS_FS_ZIP_MULTIREF
-skip:
- /* currently unimplemented */
- BUG();
-#else
work = z_erofs_vle_grab_primary_work(grp);
-#endif
work->pageofs = f->pageofs;
mutex_init(&work->lock);
@@ -797,9 +785,7 @@ static int z_erofs_vle_unzip(struct super_block *sb,
struct z_erofs_pagevec_ctor ctor;
unsigned int nr_pages;
-#ifndef CONFIG_EROFS_FS_ZIP_MULTIREF
unsigned int sparsemem_pages = 0;
-#endif
struct page *pages_onstack[Z_EROFS_VLE_VMAP_ONSTACK_PAGES];
struct page **pages, **compressed_pages, *page;
unsigned int i, llen;
@@ -811,11 +797,7 @@ static int z_erofs_vle_unzip(struct super_block *sb,
int err;
might_sleep();
-#ifndef CONFIG_EROFS_FS_ZIP_MULTIREF
work = z_erofs_vle_grab_primary_work(grp);
-#else
- BUG();
-#endif
BUG_ON(!READ_ONCE(work->nr_pages));
mutex_lock(&work->lock);
@@ -866,13 +848,11 @@ static int z_erofs_vle_unzip(struct super_block *sb,
pagenr = z_erofs_onlinepage_index(page);
BUG_ON(pagenr >= nr_pages);
-
-#ifndef CONFIG_EROFS_FS_ZIP_MULTIREF
BUG_ON(pages[pagenr] != NULL);
- ++sparsemem_pages;
-#endif
+
pages[pagenr] = page;
}
+ sparsemem_pages = i;
z_erofs_pagevec_ctor_exit(&ctor, true);
@@ -902,10 +882,8 @@ static int z_erofs_vle_unzip(struct super_block *sb,
pagenr = z_erofs_onlinepage_index(page);
BUG_ON(pagenr >= nr_pages);
-#ifndef CONFIG_EROFS_FS_ZIP_MULTIREF
BUG_ON(pages[pagenr] != NULL);
++sparsemem_pages;
-#endif
pages[pagenr] = page;
overlapped = true;
@@ -931,12 +909,10 @@ static int z_erofs_vle_unzip(struct super_block *sb,
if (err != -ENOTSUPP)
goto out_percpu;
-#ifndef CONFIG_EROFS_FS_ZIP_MULTIREF
if (sparsemem_pages >= nr_pages) {
BUG_ON(sparsemem_pages > nr_pages);
goto skip_allocpage;
}
-#endif
for (i = 0; i < nr_pages; ++i) {
if (pages[i] != NULL)
@@ -945,9 +921,7 @@ static int z_erofs_vle_unzip(struct super_block *sb,
pages[i] = __stagingpage_alloc(page_pool, GFP_NOFS);
}
-#ifndef CONFIG_EROFS_FS_ZIP_MULTIREF
skip_allocpage:
-#endif
vout = erofs_vmap(pages, nr_pages);
err = z_erofs_vle_unzip_vmap(compressed_pages,
diff --git a/drivers/staging/erofs/unzip_vle.h
b/drivers/staging/erofs/unzip_vle.h
index 3939985..3316bc3 100644
--- a/drivers/staging/erofs/unzip_vle.h
+++ b/drivers/staging/erofs/unzip_vle.h
@@ -47,13 +47,6 @@ static inline bool z_erofs_gather_if_stagingpage(struct
list_head *page_pool,
#define Z_EROFS_VLE_INLINE_PAGEVECS 3
struct z_erofs_vle_work {
- /* struct z_erofs_vle_work *left, *right; */
-
-#ifdef CONFIG_EROFS_FS_ZIP_MULTIREF
- struct list_head list;
-
- atomic_t refcount;
-#endif
struct mutex lock;
/* I: decompression offset in page */
@@ -107,10 +100,8 @@ static inline void z_erofs_vle_set_workgrp_fmt(
grp->flags = fmt | (grp->flags & ~Z_EROFS_VLE_WORKGRP_FM
[PATCH 4/6] staging: erofs: cleanup `z_erofs_vle_normalaccess_readpages'
This patch introduces `__should_decompress_synchronously' to
cleanup `z_erofs_vle_normalaccess_readpages'.
Signed-off-by: Gao Xiang
---
drivers/staging/erofs/internal.h | 11 +++
drivers/staging/erofs/super.c | 5 +
drivers/staging/erofs/unzip_vle.c | 20 ++--
3 files changed, 22 insertions(+), 14 deletions(-)
diff --git a/drivers/staging/erofs/internal.h b/drivers/staging/erofs/internal.h
index cfcc6db..c84eb97 100644
--- a/drivers/staging/erofs/internal.h
+++ b/drivers/staging/erofs/internal.h
@@ -95,6 +95,9 @@ struct erofs_sb_info {
/* the dedicated workstation for compression */
struct radix_tree_root workstn_tree;
+ /* threshold for decompression synchronously */
+ unsigned int max_sync_decompress_pages;
+
#ifdef EROFS_FS_HAS_MANAGED_CACHE
struct inode *managed_cache;
#endif
@@ -273,6 +276,14 @@ extern int erofs_try_to_free_cached_page(struct
address_space *mapping,
struct page *page);
#endif
+#define DEFAULT_MAX_SYNC_DECOMPRESS_PAGES 4
+
+static inline bool __should_decompress_synchronously(struct erofs_sb_info *sbi,
+unsigned int nr)
+{
+ return nr <= sbi->max_sync_decompress_pages;
+}
+
#endif
/* we strictly follow PAGE_SIZE and no buffer head yet */
diff --git a/drivers/staging/erofs/super.c b/drivers/staging/erofs/super.c
index 802202c..5b87f59 100644
--- a/drivers/staging/erofs/super.c
+++ b/drivers/staging/erofs/super.c
@@ -162,6 +162,11 @@ static void erofs_build_fault_attr(struct erofs_sb_info
*sbi,
static void default_options(struct erofs_sb_info *sbi)
{
+ /* set up some FS parameters */
+#ifdef CONFIG_EROFS_FS_ZIP
+ sbi->max_sync_decompress_pages = DEFAULT_MAX_SYNC_DECOMPRESS_PAGES;
+#endif
+
#ifdef CONFIG_EROFS_FS_XATTR
set_opt(sbi, XATTR_USER);
#endif
diff --git a/drivers/staging/erofs/unzip_vle.c
b/drivers/staging/erofs/unzip_vle.c
index 9fe18cd3..337a82d 100644
--- a/drivers/staging/erofs/unzip_vle.c
+++ b/drivers/staging/erofs/unzip_vle.c
@@ -1308,12 +1308,14 @@ static int z_erofs_vle_normalaccess_readpage(struct
file *file,
return 0;
}
-static inline int __z_erofs_vle_normalaccess_readpages(
- struct file *filp,
- struct address_space *mapping,
- struct list_head *pages, unsigned int nr_pages, bool sync)
+static int z_erofs_vle_normalaccess_readpages(struct file *filp,
+ struct address_space *mapping,
+ struct list_head *pages,
+ unsigned int nr_pages)
{
struct inode *const inode = mapping->host;
+ struct erofs_sb_info *const sbi = EROFS_I_SB(inode);
+ const bool sync = __should_decompress_synchronously(sbi, nr_pages);
struct z_erofs_vle_frontend f = VLE_FRONTEND_INIT(inode);
gfp_t gfp = mapping_gfp_constraint(mapping, GFP_KERNEL);
@@ -1372,16 +1374,6 @@ static inline int __z_erofs_vle_normalaccess_readpages(
return 0;
}
-static int z_erofs_vle_normalaccess_readpages(
- struct file *filp,
- struct address_space *mapping,
- struct list_head *pages, unsigned int nr_pages)
-{
- return __z_erofs_vle_normalaccess_readpages(filp,
- mapping, pages, nr_pages,
- nr_pages < 4 /* sync */);
-}
-
const struct address_space_operations z_erofs_vle_normalaccess_aops = {
.readpage = z_erofs_vle_normalaccess_readpage,
.readpages = z_erofs_vle_normalaccess_readpages,
--
1.9.1
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH 5/6] staging: erofs: add some comments for xattr subsystem
As Dan Carpenter pointed out, it is better to document what
return values of these callbacks in `struct xattr_iter_handlers'
mean and why it->ofs is increased regardless of success or
failure in `xattr_foreach'.
Suggested-by: Dan Carpenter
Signed-off-by: Gao Xiang
---
drivers/staging/erofs/xattr.c | 13 -
1 file changed, 12 insertions(+), 1 deletion(-)
diff --git a/drivers/staging/erofs/xattr.c b/drivers/staging/erofs/xattr.c
index 4942ca1..7b1367e 100644
--- a/drivers/staging/erofs/xattr.c
+++ b/drivers/staging/erofs/xattr.c
@@ -109,6 +109,13 @@ static int init_inode_xattrs(struct inode *inode)
return 0;
}
+/*
+ * the general idea for these return values is
+ * if0 is returned, go on processing the current xattr;
+ * 1 (> 0) is returned, skip this round to process the next xattr;
+ *-err (< 0) is returned, an error (maybe ENOXATTR) occurred
+ *and need to be handled
+ */
struct xattr_iter_handlers {
int (*entry)(struct xattr_iter *, struct erofs_xattr_entry *);
int (*name)(struct xattr_iter *, unsigned int, char *, unsigned int);
@@ -164,6 +171,10 @@ static int inline_xattr_iter_begin(struct xattr_iter *it,
return vi->xattr_isize - xattr_header_sz;
}
+/*
+ * Regardless of success or failure, `xattr_foreach' will end up with
+ * `ofs' pointing to the next xattr item rather than an arbitrary position.
+ */
static int xattr_foreach(struct xattr_iter *it,
const struct xattr_iter_handlers *op, unsigned int *tlimit)
{
@@ -255,7 +266,7 @@ static int xattr_foreach(struct xattr_iter *it,
}
out:
- /* we assume that ofs is aligned with 4 bytes */
+ /* xattrs should be 4-byte aligned (on-disk constraint) */
it->ofs = EROFS_XATTR_ALIGN(it->ofs);
return err;
}
--
1.9.1
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH 6/6] staging: erofs: simplify return value of `xattr_foreach'
As Dan Carpenter pointed out, there is no need to propagate positive
return values back to its callers.
Suggested-by: Dan Carpenter
Signed-off-by: Gao Xiang
---
drivers/staging/erofs/xattr.c | 24 +---
1 file changed, 9 insertions(+), 15 deletions(-)
diff --git a/drivers/staging/erofs/xattr.c b/drivers/staging/erofs/xattr.c
index 7b1367e..80dca6a 100644
--- a/drivers/staging/erofs/xattr.c
+++ b/drivers/staging/erofs/xattr.c
@@ -268,7 +268,7 @@ static int xattr_foreach(struct xattr_iter *it,
out:
/* xattrs should be 4-byte aligned (on-disk constraint) */
it->ofs = EROFS_XATTR_ALIGN(it->ofs);
- return err;
+ return err < 0 ? err : 0;
}
struct getxattr_iter {
@@ -333,15 +333,12 @@ static int inline_getxattr(struct inode *inode, struct
getxattr_iter *it)
remaining = ret;
while (remaining) {
ret = xattr_foreach(&it->it, &find_xattr_handlers, &remaining);
- if (ret >= 0)
- break;
-
- if (ret != -ENOATTR)/* -ENOMEM, -EIO, etc. */
+ if (ret != -ENOATTR)
break;
}
xattr_iter_end_final(&it->it);
- return ret < 0 ? ret : it->buffer_size;
+ return ret ? ret : it->buffer_size;
}
static int shared_getxattr(struct inode *inode, struct getxattr_iter *it)
@@ -371,16 +368,13 @@ static int shared_getxattr(struct inode *inode, struct
getxattr_iter *it)
}
ret = xattr_foreach(&it->it, &find_xattr_handlers, NULL);
- if (ret >= 0)
- break;
-
- if (ret != -ENOATTR)/* -ENOMEM, -EIO, etc. */
+ if (ret != -ENOATTR)
break;
}
if (vi->xattr_shared_count)
xattr_iter_end_final(&it->it);
- return ret < 0 ? ret : it->buffer_size;
+ return ret ? ret : it->buffer_size;
}
static bool erofs_xattr_user_list(struct dentry *dentry)
@@ -567,11 +561,11 @@ static int inline_listxattr(struct listxattr_iter *it)
remaining = ret;
while (remaining) {
ret = xattr_foreach(&it->it, &list_xattr_handlers, &remaining);
- if (ret < 0)
+ if (ret)
break;
}
xattr_iter_end_final(&it->it);
- return ret < 0 ? ret : it->buffer_ofs;
+ return ret ? ret : it->buffer_ofs;
}
static int shared_listxattr(struct listxattr_iter *it)
@@ -601,13 +595,13 @@ static int shared_listxattr(struct listxattr_iter *it)
}
ret = xattr_foreach(&it->it, &list_xattr_handlers, NULL);
- if (ret < 0)
+ if (ret)
break;
}
if (vi->xattr_shared_count)
xattr_iter_end_final(&it->it);
- return ret < 0 ? ret : it->buffer_ofs;
+ return ret ? ret : it->buffer_ofs;
}
ssize_t erofs_listxattr(struct dentry *dentry,
--
1.9.1
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH 07/29] staging: wilc1000: use 'void' return for wilc_wlan_txq_add_to_head()
Use 'void' return for wilc_wlan_txq_add_to_head() as its always
return '0' value.
Signed-off-by: Ajay Singh
---
drivers/staging/wilc1000/wilc_wlan.c | 11 +++
1 file changed, 3 insertions(+), 8 deletions(-)
diff --git a/drivers/staging/wilc1000/wilc_wlan.c
b/drivers/staging/wilc1000/wilc_wlan.c
index 590a51c..8057db9 100644
--- a/drivers/staging/wilc1000/wilc_wlan.c
+++ b/drivers/staging/wilc1000/wilc_wlan.c
@@ -71,8 +71,8 @@ static void wilc_wlan_txq_add_to_tail(struct net_device *dev,
complete(&wilc->txq_event);
}
-static int wilc_wlan_txq_add_to_head(struct wilc_vif *vif,
-struct txq_entry_t *tqe)
+static void wilc_wlan_txq_add_to_head(struct wilc_vif *vif,
+ struct txq_entry_t *tqe)
{
unsigned long flags;
struct wilc *wilc = vif->wilc;
@@ -87,8 +87,6 @@ static int wilc_wlan_txq_add_to_head(struct wilc_vif *vif,
spin_unlock_irqrestore(&wilc->txq_spinlock, flags);
mutex_unlock(&wilc->txq_add_to_head_cs);
complete(&wilc->txq_event);
-
- return 0;
}
#define NOT_TCP_ACK(-1)
@@ -275,10 +273,7 @@ static int wilc_wlan_txq_add_cfg_pkt(struct wilc_vif *vif,
u8 *buffer,
tqe->priv = NULL;
tqe->ack_idx = NOT_TCP_ACK;
- if (wilc_wlan_txq_add_to_head(vif, tqe)) {
- kfree(tqe);
- return 0;
- }
+ wilc_wlan_txq_add_to_head(vif, tqe);
return 1;
}
--
2.7.4
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH 04/29] staging: wilc1000: change return type to 'void' for wilc_deinit_host_int()
Cleanup patch to use 'void' return type for wilc_deinit_host_int(),
as its return value is not used in caller.
Signed-off-by: Ajay Singh
---
drivers/staging/wilc1000/wilc_wfi_cfgoperations.c | 4 +---
drivers/staging/wilc1000/wilc_wfi_cfgoperations.h | 2 +-
2 files changed, 2 insertions(+), 4 deletions(-)
diff --git a/drivers/staging/wilc1000/wilc_wfi_cfgoperations.c
b/drivers/staging/wilc1000/wilc_wfi_cfgoperations.c
index 37c26d4..02a8846 100644
--- a/drivers/staging/wilc1000/wilc_wfi_cfgoperations.c
+++ b/drivers/staging/wilc1000/wilc_wfi_cfgoperations.c
@@ -2175,7 +2175,7 @@ int wilc_init_host_int(struct net_device *net)
return ret;
}
-int wilc_deinit_host_int(struct net_device *net)
+void wilc_deinit_host_int(struct net_device *net)
{
int ret;
struct wilc_priv *priv = wdev_priv(net->ieee80211_ptr);
@@ -2192,8 +2192,6 @@ int wilc_deinit_host_int(struct net_device *net)
if (ret)
netdev_err(net, "Error while deinitializing host interface\n");
-
- return ret;
}
void wilc_free_wiphy(struct net_device *net)
diff --git a/drivers/staging/wilc1000/wilc_wfi_cfgoperations.h
b/drivers/staging/wilc1000/wilc_wfi_cfgoperations.h
index be412b6..1858f56 100644
--- a/drivers/staging/wilc1000/wilc_wfi_cfgoperations.h
+++ b/drivers/staging/wilc1000/wilc_wfi_cfgoperations.h
@@ -11,7 +11,7 @@
struct wireless_dev *wilc_create_wiphy(struct net_device *net,
struct device *dev);
void wilc_free_wiphy(struct net_device *net);
-int wilc_deinit_host_int(struct net_device *net);
+void wilc_deinit_host_int(struct net_device *net);
int wilc_init_host_int(struct net_device *net);
void wilc_wfi_monitor_rx(u8 *buff, u32 size);
int wilc_wfi_deinit_mon_interface(void);
--
2.7.4
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH 02/29] staging: wilc1000: change return type to 'void' for wilc_wlan_set_bssid()
Cleanup patch to use 'void' return type for wilc_wlan_set_bssid(),
as its always returns the same value.
Signed-off-by: Ajay Singh
---
drivers/staging/wilc1000/linux_wlan.c | 4 +---
drivers/staging/wilc1000/wilc_wfi_netdevice.h | 2 +-
2 files changed, 2 insertions(+), 4 deletions(-)
diff --git a/drivers/staging/wilc1000/linux_wlan.c
b/drivers/staging/wilc1000/linux_wlan.c
index 49afda6..d1d2c64 100644
--- a/drivers/staging/wilc1000/linux_wlan.c
+++ b/drivers/staging/wilc1000/linux_wlan.c
@@ -192,14 +192,12 @@ static struct net_device *get_if_handler(struct wilc
*wilc, u8 *mac_header)
return NULL;
}
-int wilc_wlan_set_bssid(struct net_device *wilc_netdev, u8 *bssid, u8 mode)
+void wilc_wlan_set_bssid(struct net_device *wilc_netdev, u8 *bssid, u8 mode)
{
struct wilc_vif *vif = netdev_priv(wilc_netdev);
memcpy(vif->bssid, bssid, 6);
vif->mode = mode;
-
- return 0;
}
int wilc_wlan_get_num_conn_ifcs(struct wilc *wilc)
diff --git a/drivers/staging/wilc1000/wilc_wfi_netdevice.h
b/drivers/staging/wilc1000/wilc_wfi_netdevice.h
index 1837808..30151b2 100644
--- a/drivers/staging/wilc1000/wilc_wfi_netdevice.h
+++ b/drivers/staging/wilc1000/wilc_wfi_netdevice.h
@@ -215,6 +215,6 @@ void wilc_netdev_cleanup(struct wilc *wilc);
int wilc_netdev_init(struct wilc **wilc, struct device *dev, int io_type,
const struct wilc_hif_func *ops);
void wilc_wfi_mgmt_rx(struct wilc *wilc, u8 *buff, u32 size);
-int wilc_wlan_set_bssid(struct net_device *wilc_netdev, u8 *bssid, u8 mode);
+void wilc_wlan_set_bssid(struct net_device *wilc_netdev, u8 *bssid, u8 mode);
#endif
--
2.7.4
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH 05/29] staging: wilc1000: change return type to 'void' for wilc_wfi_deinit_mon_interface()
Use 'void' return type for wilc_wfi_deinit_mon_interface(), as same
value always return.
Signed-off-by: Ajay Singh
---
drivers/staging/wilc1000/linux_mon.c | 3 +--
drivers/staging/wilc1000/wilc_wfi_cfgoperations.h | 2 +-
2 files changed, 2 insertions(+), 3 deletions(-)
diff --git a/drivers/staging/wilc1000/linux_mon.c
b/drivers/staging/wilc1000/linux_mon.c
index 1afdb9e..a634468 100644
--- a/drivers/staging/wilc1000/linux_mon.c
+++ b/drivers/staging/wilc1000/linux_mon.c
@@ -253,7 +253,7 @@ struct net_device *wilc_wfi_init_mon_interface(const char
*name,
return wilc_wfi_mon;
}
-int wilc_wfi_deinit_mon_interface(void)
+void wilc_wfi_deinit_mon_interface(void)
{
bool rollback_lock = false;
@@ -270,5 +270,4 @@ int wilc_wfi_deinit_mon_interface(void)
}
wilc_wfi_mon = NULL;
}
- return 0;
}
diff --git a/drivers/staging/wilc1000/wilc_wfi_cfgoperations.h
b/drivers/staging/wilc1000/wilc_wfi_cfgoperations.h
index 1858f56..4812c8e 100644
--- a/drivers/staging/wilc1000/wilc_wfi_cfgoperations.h
+++ b/drivers/staging/wilc1000/wilc_wfi_cfgoperations.h
@@ -14,7 +14,7 @@ void wilc_free_wiphy(struct net_device *net);
void wilc_deinit_host_int(struct net_device *net);
int wilc_init_host_int(struct net_device *net);
void wilc_wfi_monitor_rx(u8 *buff, u32 size);
-int wilc_wfi_deinit_mon_interface(void);
+void wilc_wfi_deinit_mon_interface(void);
struct net_device *wilc_wfi_init_mon_interface(const char *name,
struct net_device *real_dev);
void wilc_mgmt_frame_register(struct wiphy *wiphy, struct wireless_dev *wdev,
--
2.7.4
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH 01/29] staging: wilc1000: change return type to 'void' for wilc_frame_register()
Cleanup patch to use 'void' return type for wilc_frame_register(), as
its return value is not used.
Signed-off-by: Ajay Singh
---
drivers/staging/wilc1000/host_interface.c | 6 ++
drivers/staging/wilc1000/host_interface.h | 2 +-
2 files changed, 3 insertions(+), 5 deletions(-)
diff --git a/drivers/staging/wilc1000/host_interface.c
b/drivers/staging/wilc1000/host_interface.c
index 5388be9..7729f83 100644
--- a/drivers/staging/wilc1000/host_interface.c
+++ b/drivers/staging/wilc1000/host_interface.c
@@ -3641,14 +3641,14 @@ int wilc_listen_state_expired(struct wilc_vif *vif, u32
session_id)
return result;
}
-int wilc_frame_register(struct wilc_vif *vif, u16 frame_type, bool reg)
+void wilc_frame_register(struct wilc_vif *vif, u16 frame_type, bool reg)
{
int result;
struct host_if_msg *msg;
msg = wilc_alloc_work(vif, handle_register_frame, false);
if (IS_ERR(msg))
- return PTR_ERR(msg);
+ return;
switch (frame_type) {
case ACTION:
@@ -3670,8 +3670,6 @@ int wilc_frame_register(struct wilc_vif *vif, u16
frame_type, bool reg)
netdev_err(vif->ndev, "%s: enqueue work failed\n", __func__);
kfree(msg);
}
-
- return result;
}
int wilc_add_beacon(struct wilc_vif *vif, u32 interval, u32 dtim_period,
diff --git a/drivers/staging/wilc1000/host_interface.h
b/drivers/staging/wilc1000/host_interface.h
index a48818f..15ffaeb 100644
--- a/drivers/staging/wilc1000/host_interface.h
+++ b/drivers/staging/wilc1000/host_interface.h
@@ -348,7 +348,7 @@ int wilc_remain_on_channel(struct wilc_vif *vif, u32
session_id,
wilc_remain_on_chan_ready ready,
void *user_arg);
int wilc_listen_state_expired(struct wilc_vif *vif, u32 session_id);
-int wilc_frame_register(struct wilc_vif *vif, u16 frame_type, bool reg);
+void wilc_frame_register(struct wilc_vif *vif, u16 frame_type, bool reg);
int wilc_set_wfi_drv_handler(struct wilc_vif *vif, int index, u8 mode,
u8 ifc_id);
int wilc_set_operation_mode(struct wilc_vif *vif, u32 mode);
--
2.7.4
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
